Skip to content
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

chore(pie-monorepo): DSW-1823 update PR template to be dynamic based on scope of change #1345

Merged
merged 16 commits into from
Apr 4, 2024

Conversation

siggerzz
Copy link
Contributor

@siggerzz siggerzz commented Mar 21, 2024

Describe your changes (can list changeset entries if preferable)

This functionality won't be live on the origin PIE repo until the templates are in main, but I've tested this functionality in my fork, and you can see it in action here

Author Checklist (complete before requesting a review)

  • I have performed a self-review of my code

@siggerzz siggerzz requested review from a team as code owners March 21, 2024 14:26
Copy link

changeset-bot bot commented Mar 21, 2024

⚠️ No Changeset found

Latest commit: c5cef9d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

raoufswe
raoufswe previously approved these changes Mar 22, 2024
@ashleynolan
Copy link
Contributor

Could we have a quick catchup on this before it's merged – I think me and @xander-marjoram may have had a bit of feedback that's worth including (mine isn't big – but could be a small improvement)

@xander-marjoram
Copy link
Contributor

I like the flexibility this approach gives us (and it's a neat trick using the links to particular templates), but I wonder if it is adding complexity when it might not need to be added.

I've had an idea that we could use something like Danger to check whether there are component/docs/storybook changes and dynamically create a comment with a checklist for the author and reviewers to complete.

I think it would be a bit more involved to create, but I think the developer experience would end up being a bit nicer and there would hopefully be less room for error in terms of making sure the template has good coverage, as there may end up being PRs that update a component and the docs site at the same time, for example.

@ashleynolan
Copy link
Contributor

@xander-marjoram @siggerzz Totally agree – I think we should merge this and then iterate on it to improve it with that sort of functionality.

I imagine we'd need to use a custom Github action to add comments like that (not sure Danger would be able to do it from my experience with it) and all should be possible, but best to merge this and iterate on it.

fernandofranca
fernandofranca previously approved these changes Apr 4, 2024
Copy link
Contributor

@fernandofranca fernandofranca left a comment

Choose a reason for hiding this comment

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

Good stuff!!!

@siggerzz siggerzz merged commit b7ec2c0 into main Apr 4, 2024
33 checks passed
@siggerzz siggerzz deleted the dsw-1823-pr-template branch April 4, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants