-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update workflow documentation in contributing page #74
base: develop
Are you sure you want to change the base?
Update workflow documentation in contributing page #74
Conversation
Added you to review this update. I may have gone a bit overboard. Happy to alter in any way you see fit. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Test summaryRun details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Codecov Report
@@ Coverage Diff @@
## develop #74 +/- ##
========================================
Coverage 66.82% 66.82%
========================================
Files 12 12
Lines 853 853
Branches 196 196
========================================
Hits 570 570
Misses 229 229
Partials 54 54 Continue to review full report at Codecov.
|
CONTRIBUTING.md
Outdated
2. Do work on the `feature` or `fix` branch. When finished, submit a pull request to merge your work back into the `develop` branch. In the PR, link to the issue (see the sidebar options on the right). | ||
3. If there are conflicts between your branch and the code that now exists on `develop` branch, merge `develop` into your branch to reintegrate new changes and resolve any conflicts. | ||
4. Assign a minimum on 1 reviewer to the pull request. At least one engineer must review all PRs. | ||
5. If any updated components (`/packages`) that will need to be relased to npm exist, be sure to update package version. numbers in their respective `package.json` files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this will work well. There might be multiple PRs making changes to the same package and merging something to develop
doesn't mean it goes straight to npm. So with this workflow we might end up bumping packages version multiple times before we even release to NPM. Typically we would want to group a few PRs before releasing a new package version.
My suggestion here is to do the process of updating package versions separately once we're ready to publish a new npm release. At the feature branch stage, we only have PR authors add the CHANGELOG entries to the PR template.
Example:
- 4 PRs are merged to develop, 2 packages updated.
- Once we're ready to publish to npm, a
maintainer/owner
creates a PR againsttrunk
fromdevelop
or arelease
branch, manually update the package version to the next desired version and update the CHANGELOG.md. Then publish to npm and merge into trunk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation updated. Please review @nicholasio
CONTRIBUTING.md
Outdated
4. Create a pull request in github to merge the `release/[github-issue-number]-[release-number]` branch into `trunk`. Assign one reviewer to the PR. Add "Component Libarary Release [release number]" and "CHANGELOG" updates to the description. | ||
5. Once branch is approved, all tests pass, and merged on github, switch to the `trunk` branch locally and pull the changes you just merged. | ||
6. Verify changes are working correctly locally and, once the deploy is complete, on [Baseline](https://baseline.10up.com/components). | ||
7. If any updated components (`/packages`) that will need to be relased to npm exist, be sure to update package version numbers in their respective `package.json` files (this should have been completed in the branch workflow - see above). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7. If any updated components (`/packages`) that will need to be relased to npm exist, be sure to update package version numbers in their respective `package.json` files (this should have been completed in the branch workflow - see above). | |
7. If any updated components (`/packages`) that will need to be relased to npm exist, be sure to update package version numbers in their respective `package.json` files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation updated, please review @nicholasio
CONTRIBUTING.md
Outdated
5. Once branch is approved, all tests pass, and merged on github, switch to the `trunk` branch locally and pull the changes you just merged. | ||
6. Verify changes are working correctly locally and, once the deploy is complete, on [Baseline](https://baseline.10up.com/components). | ||
7. If any updated components (`/packages`) that will need to be relased to npm exist, be sure to update package version numbers in their respective `package.json` files (this should have been completed in the branch workflow - see above). | ||
8. If any package.json files need updating, be sure to commit the updates and push to the remote `trunk` branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be done as part of the release
branch instead of pushing to trunk
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation updated, please review @nicholasio
@nicholasio I've updated the documentation... I left some of your comments open for quick reference to the prior dialog. Please review when you have a moment. Hopefully, I have better captured your intention / workflow. |
Update workflow documentation on contributing page.
If this is too verbose / complicated - happy to trim per that guidance
Closes #74