diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 04b02781b0cb..b7d510158e85 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,110 +1,6 @@ # Contributing Guide for Artemis -Please read this guide before creating a pull request, otherwise your contribution might not be approved. +Read the [setup guide](https://docs.artemis.cit.tum.de/dev/setup.html) on how to set up your local development environment. -## Branch Organization +Before creating a pull request, please read the [guidelines to the development process](https://docs.artemis.cit.tum.de/dev/development-process.html) as well as the [coding and design guidelines](https://docs.artemis.cit.tum.de/dev/guidelines.html). -All pull request branches are created from develop. - -We use the following structure for branch names: - -\/\/\ - -Possible types are: - -- feature -- enhancement -- bugfix -- hotfix - -The pull request template will provide additional information on the requirement for the integration of changes into Artemis. -Once the changes in your pull request are approved by one of our reviewers, they can be merged into develop. - -## Pull request (PR) guidelines: - -- **Merge fast**: PRs should only be open for a couple of days. -- **Small packages**: PRs should be as small as possible and ideally concentrate on a single topic. Features should be split up into multiple PRs if it makes sense. -- **Until the PR is _ready-for-review_, the PR should be a [Draft PR](https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/about-pull-requests#draft-pull-requests)** -- **Definition of done**: Before requesting a code review make sure that the PR is _ready-for-review_: - - The PR template is filled out completely, containing as much information as needed to understand the feature. - - All tasks from the template checklist are done and checked off (writing tests, adding screenshots, etc.). - - The branch of the PR is up-to-date with develop. - - The last build of the PR is successful. - -## Code review guidelines - -- **Check out the code and test it**: Testing the feature/enhancement/bugfix helps to understand the code. -- **Respect the PR scope**: Bugfixes, enhancements or implementations that are unrelated to the PRs topic should not be enforced in a code review. -In this case the reviewer or PR maintainer needs to make sure to create an issue for this topic on GitHub or the internal task tracking tool so it is not lost. -- **Code style is not part of a code review**: Code style and linting issues are not part of the review process. If issues in code style or linting arise, the linters and auto formatters used in our CI tools need to be updated. -- **Enforce guidelines**: Enforcing technical & design guidelines is an integral part of the code review (e.g. consistent REST urls). -- **Mark optional items**: Review items that are optional from the reviewers' perspective should be marked as such (e.g. "Optional: You could also do this with...") -- **Explain your rational**: If the reviewer requests a change, the reasoning behind the change should be explained (e.g. not "Please change X to Y", but "Please change X to Y, because this would improve Z") - -## Development Workflow - -Find here [a guide](docs/dev/setup.rst) on how to setup your local development environment. - -## Route Naming Conventions - -- Always use **kebab-case** (e.g. "/exampleAssessment" → "/example-assessment") -- The routes should follow the general structure entity > entityId > sub-entity ... (e.g. "/exercises/{exerciseId}/participations") -- Use **plural for server route's** entities and **singular for client route's** entities -- Specify the key entity at the end of the route (e.g. "text-editor/participations/{participationId}" should be changed to "participations/{participationId}/text-editor") -- Never specify an id that is used only for consistency and not used in the code (e.g. GET "/courses/{courseId}/exercises/{exerciseId}/participations/{participationId}/submissions/{submissionId}" can be simplified to GET "/submissions/{submissionId}" because all other entities than the submission are either not needed or can be loaded without the need to specify the id) - -## CSS Guidelines - -We are using [Scss](https://sass-lang.com) to write modular, reusable css. - -We have a couple of global scss files in `webapp/content` but encourage [component dependent css with angular's styleUrls](https://angular.io/guide/component-styles). - -From a methodology viewpoint we encourage the use of [BEM](http://getbem.com/introduction/). -```scss -.my-container { - // container styles - &__content { - // content styles - &--modifier { - // modifier styles - } - } -} -``` - -Within the component html files, we encourage the use of [bootstrap css](https://getbootstrap.com/). - -Encouraged html styling: -`
some content
` - -## Testing - -We create unit & integration tests for the Artemis server and client. -Adding tests is an integral part of any pull request - please be aware that your pull request will not be approved until you provide automated tests for your implementation! -Our goal is to keep the test coverage above 80%. - -### Server Testing - -We use the [Spring Boot testing utilities](https://docs.spring.io/spring-boot/docs/current/reference/html/boot-features-testing.html) for server side testing. - -Location of test files: `src/test/java` - -Execution command: `./gradlew test` - -### Client Testing - -We use [Jest](https://jestjs.io/) for client side testing. - -For convenience purposes we have [Sinon](https://sinonjs.org/) and [Chai](https://www.chaijs.com/) as dependencies, so that easy stubbing/mocking is possible ([sinon-chai](https://github.com/domenic/sinon-chai)). - -Location of test files: `src/test/javascript` - -Execution command: `npm run test` - -The folder structure is further divided into: - -- component -- integration -- service - -The tests located in the folder `/app` are not working at the moment and are not included in the test runs. diff --git a/docs/dev/development-process/development-process.rst b/docs/dev/development-process/development-process.rst index c5027e19c886..b890ba99774c 100644 --- a/docs/dev/development-process/development-process.rst +++ b/docs/dev/development-process/development-process.rst @@ -88,7 +88,15 @@ Furthermore it is important to include a description of the user flow that refer -4. Implement the Feature +4. Create a local Branch +======================== +We use the following structure for branch names: ``//`` +- Possible types are: **feature**, **chore**, and **bugfix** +- Possible areas are all allowed feature tags (see :ref:`pr_naming_conventions`), written kebab-case. For example, ``Programming exercises`` becomes **programming-exercises**. + +**Branches that do not follow this structure will get rejected automatically!** + +5. Implement the Feature ======================== In this step, the development team converts the detailed plans and designs outlined in the functional proposal into working code. This step requires careful adherence to the previously defined requirements and system architecture to ensure that the function fits seamlessly into the existing system and fulfills the specified functional and performance criteria. @@ -96,11 +104,13 @@ This step requires careful adherence to the previously defined requirements and .. note:: Make sure to follow the `Artemis Code and Design Guidelines `_. -5. Create a Pull Request +6. Create a Pull Request ======================== After the feature implementation is complete, the developer is required to create a pull request for integrating the feature into the develop branch. The subsequent sections provide guidance on the naming conventions and outline the necessary steps for creating and merging a pull request. +.. _pr_naming_conventions: + Naming Conventions for GitHub Pull Requests ------------------------------------------- @@ -124,13 +134,12 @@ Naming Conventions for GitHub Pull Requests - “Fix an issue when clicking on the start exercise button” - “Add the possibility for instructors to define submission policies” - - Steps to Create and Merge a Pull Request ---------------------------------------- 0. Preconditions (For Developers Only) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +* **Merge fast**: PRs should only be open for a couple of days. * Limit yourself to one functionality per pull request. * Split up your task in multiple branches & pull requests if necessary. * `Commit Early, Commit Often, Perfect Later, Publish Once. `_ @@ -144,6 +153,8 @@ Steps to Create and Merge a Pull Request * Make sure all steps in the `Checklist `_ are completed. * Add or update the "Steps for Testing" in the description of your pull request. * Make sure that the changes in the pull request are only the ones necessary. +* Make sure that the PR is up-to-date with develop. +* Make sure at least all required checks pass. * Mark the pull request as `ready for review. `_ 3. Review Process @@ -158,10 +169,12 @@ Steps to Create and Merge a Pull Request * Perform the "Steps for Testing" and verify that the new functionality is working as expected. * Verify that related functionality is still working as expected. * Ensure that the code changes... - * conform to the code style guide. + * conform to the code style and code design guidelines. * are easily understandable. * contain thorough documentation where applicable. * maintain reasonable performance (e.g. no excessive/unnecessary database queries or HTTP calls). +* Respect the PR scope: Implementations that are unrelated to the PRs topic should not be enforced in a code review. +* Explain your rationale when requesting changes: E.g., not "Please change X to Y", but "Please change X to Y, because this would improve Z" * Submit your comments and status (✅ Approve or ❌ Request Changes) using GitHub. * Explain what you did (test, review code) and on which test server in the review comment. @@ -193,8 +206,6 @@ Steps to Create and Merge a Pull Request ^^^^^^^^^^^^^^^^ A project maintainer merges your changes into the ``develop`` branch. - - Stale Bot --------- diff --git a/docs/dev/guidelines/client.rst b/docs/dev/guidelines/client.rst index dac898809b57..3559fbe12d85 100644 --- a/docs/dev/guidelines/client.rst +++ b/docs/dev/guidelines/client.rst @@ -470,3 +470,29 @@ Best Practices: 1. Dynamic Subscription Handling: Subscribe to topics on an as-needed basis. Unsubscribe from topics that are no longer needed to keep the number of active subscriptions within the recommended limit. 2. Efficient Topic Aggregation: Use topic aggregation techniques to consolidate related data streams into a single subscription wherever possible. Consequently, don't create a new topic if an existing one can be reused. 3. Small Messages: Send small messages and use DTOs. See :ref:`server-guideline-dto-usage` for more information and examples. + +19. Styling +=========== + +We are using `Scss `_ to write modular, reusable css. We have a couple of global scss files in ``webapp/content/scss``, but encourage component dependent css using `Angular styleUrls `_. + +From a methodology viewpoint we encourage the use of `BEM `_: + +.. code-block:: scss + + .my-container { + // container styles + &__content { + // content styles + &--modifier { + // modifier styles + } + } + } + +Within the component html files, we encourage the use of `bootstrap css `_: + +.. code-block:: html + +
some content
+