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

Update README.md to include Packwerk Retrospective article #389

Closed
wants to merge 1 commit into from

Conversation

caffeinefriend
Copy link

Sid Sijbrandij sends his regards.

What are you trying to accomplish?

Adding a link to the README - https://shopify.engineering/a-packwerk-retrospective

What approach did you choose and why?

Concise

What should reviewers focus on?

Would you like this link to live in a new section called 'Related Documentation'? That made the most sense to me. :)

Type of Change

  • Bugfix
  • New feature
  • Non-breaking change (a change that doesn't alter functionality - i.e., code refactor, configs, etc.)

Additional Release Notes

  • Breaking change (fix or feature that would cause existing functionality to change)

Include any notes here to include in the release description. For example, if you selected "breaking change" above, leave notes on how users can transition to this version.

If no additional notes are necessary, delete this section or leave it unchanged.

Checklist

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • It is safe to rollback this change.

@caffeinefriend caffeinefriend requested a review from a team as a code owner February 22, 2024 20:28
Copy link
Member

@gmcgibbon gmcgibbon left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request! Please also sign the CLA if you haven't already.

@@ -85,6 +85,10 @@ Bug reports and pull requests are welcome on GitHub at https://github.com/Shopif

Read and follow the guidelines in [CONTRIBUTING.md](https://github.com/Shopify/packwerk/blob/main/CONTRIBUTING.md).

## Related Documentation
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
## Related Documentation
## Learn More

@@ -85,6 +85,10 @@ Bug reports and pull requests are welcome on GitHub at https://github.com/Shopif

Read and follow the guidelines in [CONTRIBUTING.md](https://github.com/Shopify/packwerk/blob/main/CONTRIBUTING.md).

## Related Documentation

[A Packwerk Retrospective](https://shopify.engineering/a-packwerk-retrospective) by Gannon McGibbon - 12 min read
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[A Packwerk Retrospective](https://shopify.engineering/a-packwerk-retrospective) by Gannon McGibbon - 12 min read
[A Packwerk Retrospective](https://railsatscale.com/2024-01-26-a-packwerk-retrospective/) by Gannon McGibbon and Chris Salzberg

@caffeinefriend
Copy link
Author

Happy to contribute @gmcgibbon (Also Sid asked me to 🙂).

Can you help me understand how to sign the CLA for you? :
https://github.com/Shopify/packwerk/blob/main/.github/workflows/cla.yml

@exterm
Copy link
Contributor

exterm commented Feb 23, 2024

mmmh, should this section then include all shopify-published material related to packwerk?

https://shopify.engineering/enforcing-modularity-rails-apps-packwerk for example?

If this is about background information, it probably then should also include the two blog posts on Componentization?

🤷🏼

@gmcgibbon
Copy link
Member

@caffeinefriend If you click on the details for the CLA check failure it tells you how to sign.

@caffeinefriend
Copy link
Author

CLA signed ✅

@caffeinefriend
Copy link
Author

I have signed the CLA!

@rafaelfranca
Copy link
Member

Thank you for the pull request. I don't think we need to link to external article in the README. Those articles get old and change and might no reflect what the tooling is anymore.

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.

None yet

4 participants