Skip to content

feat(template): moves tests & linter fixes#3

Open
vic-unimedia wants to merge 1 commit into
mainfrom
feature/MMC-188-pr-template-testing-improvements
Open

feat(template): moves tests & linter fixes#3
vic-unimedia wants to merge 1 commit into
mainfrom
feature/MMC-188-pr-template-testing-improvements

Conversation

@vic-unimedia
Copy link
Copy Markdown

@vic-unimedia vic-unimedia commented Nov 3, 2022

IMPORTANT

In order to help other team mates in having context of the ticket and knowing which work has been made, it's mandatory to fill all the mandatory (*) sections (that includes replacing the text placeholders to describe properly the ticket). If any of them have not been filled in, the PR is not ready to review, so people will wait to do the review. (remove this note when you're sure all of the reqs. are fulfilled)

(*)Description

Moves testing part to the end in order to avoid clogging PR body with test images & fixes Markdown linter

(*)Type of change

  • New feature (non-breaking change which adds functionality)

(*)How Has This Been Tested?

Please describe the tests that you ran to verify your changes. For each of the usecases, provide instructions so the team can understand what's been solved in this ticket. Please provide screenshots for each of them, so we know how do you tested for endpoints, bugfixes, etc which are exposed independently if it's an internal GRPC service, public service, etc.

An assertment such as 'make test' shows the test pass will only be enough if we're unit testing a task which was broken into smaller unit pieces (i.e. repository, use-case, controller, etc that still is not coupled to the server) that are not coupled/exposed yet. The rest of the tickets will include screenshots showing how the functionality was tested.

  • Test A

(*)Checklist:

  • (*)Branch name follows our guidelines naming convention
{{feature|bugfix|hotfix|docs}}/MMC-{{ticket_id_on_linear}}-{{short_title}}
  • (*)The commit message follows our guideline convention
{{feat|docs|style|refactor|test|chore|fix|hotfix}}({{scope}}): {{short_title}}    (**NOTE** scope is usually the name of the micro)

{{ticket_description}}

resolves: MMC-{{ticket_id_on_linear}}
  • This ticket requires business to validate its resolution before being able to release
  • (*)My code follows the style guidelines of this project
  • [ x (*)I have performed a self-review of my own code
  • (*)I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (if needed)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • (*)New and existing unit tests pass locally with my changes
  • (*)I've reviewed and ensured all dependant components and config have been added and once merged, the functionality can be deployed to any environment
  • (*)I'll remember to squash all the commits (if multiple) when merging to develop. In case of a PR against main, I'll merge main into develop first and once the PR is approved, I'll just merge (no squash)

Moves checklist above tests so test images don't bloat PR description
@vic-unimedia vic-unimedia force-pushed the feature/MMC-188-pr-template-testing-improvements branch from 71262ea to dba5552 Compare November 3, 2022 11:18
In order to help other team mates in having context of the ticket and knowing which work has been made, it's mandatory to fill all the mandatory (*) sections (that includes replacing the text placeholders to describe properly the ticket). If any of them have not been filled in, the PR is not ready to review, so people will wait to do the review. (remove this note when you're sure all of the reqs. are fulfilled)

## (*)Description
## **Description**
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to keep the () here and in the other places you removed it, as the description above explains that fields highlighted with the () are mandatory to be filled, so it's important we let people know the "must fill in"s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants