Conversation
|
|
||
| ## Core dependencies | ||
|
|
||
| RDMO is a [Django](https://www.djangoproject.com/) application with an integrated [React](https://react.dev/) backend. |
There was a problem hiding this comment.
Do you mean integrated React frontend?
|
|
||
| ### Python and Django | ||
|
|
||
| For Python code, we use [ruff](https://github.com/astral-sh/ruff) as linter (as configured in `pyproject.toml`) and follow its suggestions. Unlike many Python projects, we do not enforce [black](https://github.com/psf/black) or [ruff format](https://github.com/astral-sh/ruff). Contributors are trusted to follow the style guidelines without automated formatting. |
There was a problem hiding this comment.
If this is really up for discussion: I do not agree that this is a good idea. I would vote in favor for auto-formatting.
There was a problem hiding this comment.
After learning about https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma yesterday, I am looking into it again. It seems other style changes already moved RDMO closer to black (parenthesis for multiline statements), so my biggest problems with black seem to vanish. I need to look at the diffs again, but I am not as opposed as I was two years ago.
|
|
||
| We only use functional components and only one component should be in a single file. | ||
|
|
||
| ## UI/UI considerations |
|
|
||
| Custom CSS code should be kept to a minimum. For CSS classes hyphens `-` should be used instead of underscores (`_`). | ||
|
|
||
| In the interactive front end, `a` should be used when there is a front end route or a backend URL which users might want to copy or bookmark. In all other cases, a `button` is preferred. The CSS class `link` can be used to make it look like a link. Usually `button` has `type="button"`, unless used as submit button in a form. |
There was a problem hiding this comment.
Better: "In the interactive front end, links using the a element only be used when ..."
There was a problem hiding this comment.
Even better: "In the interactive front end, links using an anchor (a element) should be used only when ..."
|
|
||
| In the interactive front end, `a` should be used when there is a front end route or a backend URL which users might want to copy or bookmark. In all other cases, a `button` is preferred. The CSS class `link` can be used to make it look like a link. Usually `button` has `type="button"`, unless used as submit button in a form. | ||
|
|
||
| Form input fields should should apply or save automatically. This save operation needs a visual indicator to communicate success to the user. An exception are fields in models modals are only saved on the submit button of the modal. |
There was a problem hiding this comment.
Better: "are fields in modals, which are only saved ..."
There was a problem hiding this comment.
doubled "should" in line 102: should should
"models modals" correct also in line 102?
| Automatic tests are very important to us and we require tests for each new feature implemented. Usually we implement and run integration tests, which perform single requests against a URL or endpoint. Most crucial are tests, which perform requests as different users with different permissions. If you intend to add tests for your contributed code, we recommend looking at the existing tests to get a better understanding of our testing approach. | ||
|
|
||
| These checks will run as a CI job as well. | ||
| How to run the tests is described in the [Testing docs of the RDMO documentation](https://rdmo.readthedocs.io/en/latest/development/index.html) |
There was a problem hiding this comment.
I think the link should be
https://rdmo.readthedocs.io/en/latest/development/testing.html
| ```console | ||
| $ pytest | ||
| ``` | ||
| Ideally, commits should made often, ideally after every small, logical unit of work. Keep the later use in e.g. `git blame` in mind. Commit messages should be clear and concise and should use the imperative mood, be capitalized, not end in a period, and not exceed 72 characters. If a longer messages is needed, separate subject and body by an empty line (see also [How to Write a Git Commit Message](https://cbea.ms/git-commit/)). |
There was a problem hiding this comment.
Don't we want the number of the related issue contained?
|
|
||
| Consistent and descriptive naming is one of the key factors for readable and maintainable code. Giving good names to variables, functions, and classes not only makes the code easier to understand and debug, it also helps when reviewing code and onboarding new team members. | ||
|
|
||
| In general, we want to follow established naming conventions used in [Django](https://www.djangoproject.com), [Django Rest Framework](https://www.django-rest-framework.org) (DRF), [React](https://react.dev) and [Bootstrap](https://getbootstrap.com). In addition, please follow the guidelines below when naming new variables, functions or methods: |
There was a problem hiding this comment.
I'd expect the links to point to the naming conventions, not the overview pages.
|
|
||
| In the interactive front end, `a` should be used when there is a front end route or a backend URL which users might want to copy or bookmark. In all other cases, a `button` is preferred. The CSS class `link` can be used to make it look like a link. Usually `button` has `type="button"`, unless used as submit button in a form. | ||
|
|
||
| Form input fields should should apply or save automatically. This save operation needs a visual indicator to communicate success to the user. An exception are fields in models modals are only saved on the submit button of the modal. |
There was a problem hiding this comment.
doubled "should" in line 102: should should
"models modals" correct also in line 102?
Just for the record I would like to clarify: There has been a contributor guide in this repo since 2018. This PR updates the guide. |
|
Question: Is this repo the best place for these documents? Should they apply to plugins as well? Would a central place, like the docs, make more sense? |
|
@afuetterer yes, My idea was that this repo is actually the place where people would look. I expect that We follow KITODO here: https://github.com/UB-Mannheim/kitodo-coding-guidelines. |
|
|
||
| In general, we want to follow established naming conventions used in [Django](https://www.djangoproject.com), [Django Rest Framework](https://www.django-rest-framework.org) (DRF), [React](https://react.dev) and [Bootstrap](https://getbootstrap.com). In addition, please follow the guidelines below when naming new variables, functions or methods: | ||
|
|
||
| * The prefixes `set` and `get` should be used for functions which perform little to none operations. In Python classes, we prefer `@property` instead of `get_egg(self)`. For more complex functions we use `compute` as prefix. An exception are well established Django/DRF patterns like `get_queryset`. |
There was a problem hiding this comment.
After conversation with @CalamityC : This part should be more detailed and it should be clearer what is for JS what is for Python and what is for both. The @property part probably belongs the the Pyhton patterns section below.
This PR adds/updates 3 documents to the repo:
CONTRIBUTING.mdcontains the guidelines for contributors how we work.ARCHITECTURE.mdaims to explain the structure of the repository, file naming conventions and external and internal dependencies.STYLE.mdcollects naming conventions, code patterns and ux/ui considerations.I will present the documents at the community meeting and we will have a review process with the community. While afterwards
CONTRIBUTING.mdis meant to be more or less stable, I expect the other two documents to be updated more frequently.Direct links for convenience: