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

Don't allow update on specific files #272

Merged
merged 1 commit into from
Dec 2, 2022
Merged

Conversation

gmcgibbon
Copy link
Member

What are you trying to accomplish?

When updating the todo, Packwerk should know about all packages in an application. Most of the time, running update-todo with arguments doesn't make sense and is prone to errors. So, let's stop this usage and make sure developers are always updating todos without arguments.

What approach did you choose and why?

Pass the FilesForProcessing object instead of the file set specifically to provide more info about the file-set, and to lazily fetch / filter the files.

What should reviewers focus on?

Does this make sense?

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.

When updating the todo, Packwerk should know about all packages in an
application. Most of the time, running update-todo with arguments
doesn't make sense and is prone to errors. So, let's stop this usage and
make sure developers are always updating todos without arguments.
Copy link
Contributor

@alexevanczuk alexevanczuk left a comment

Choose a reason for hiding this comment

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

Seems good to me, thanks for doing this!

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.

2 participants