Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Changelog.md and Danger Github action to code review process #3258

Merged
merged 5 commits into from
Dec 8, 2022

Conversation

pengyin-shan
Copy link
Contributor

Fixes #3257

1. Add Danger gem and Github action to the roadmap

Note that Danger is for reference. We can discuss changing the Danger configuration. Currently, the suggestion/reminder is:

  1. PR prefers <= 1000 lines
  2. If PR change >= 50, suggest adding a test
  3. Remind to update CHANGELOG
  4. [WIP] in the title will make this PR to be a draft pr
  5. #trivial means no changelog is needed

An example:
Screenshot 2022-12-06 at 4 06 56 PM

2. CHANGELOG.md is ready for use

The workflow would be to add the description and link to the issue (if any) to the CHANGELOG every time a PR is submitted. I created the 'Changed', 'Added' and 'Fixed' sections for categorization purposes, but we are feeling to change to other formats.
The person who's gonna do the release will add the release version as the header to the CHANGELOG. Later, developers will pull the CHANGELOG and continue to build above it.

Example:
Screenshot 2022-12-06 at 5 08 35 PM

By doing this, we will have a good record of the process, and whoever doing the release can use CHANGELOG as part of the release note.

We can also consider adding this step to the contribution guide.

Copy link
Contributor

@benjaminfaure benjaminfaure left a comment

Choose a reason for hiding this comment

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

Looking good, thank you.

Because it includes the CHANGELOG file, should we merge this one as soon as possible ?

Dangerfile Outdated Show resolved Hide resolved
Copy link
Contributor

@briri briri left a comment

Choose a reason for hiding this comment

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

looks good Wendy. I'm just wondering if the test check should use the RSpec directory instead of /test/

@briri
Copy link
Contributor

briri commented Dec 8, 2022

and agree with @benjaminfaure that once this is ready we should merge it so we can start using the Changelog immediately

@pengyin-shan
Copy link
Contributor Author

Thank you @benjaminfaure @briri ! Will merge after all tests passed

@pengyin-shan pengyin-shan merged commit aeba6b5 into development Dec 8, 2022
@pengyin-shan pengyin-shan deleted the changelog_danger branch December 8, 2022 14:16
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants