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

feat: add --set-exit-if-changed on pub upgrade #4146

Open
alestiago opened this issue Feb 26, 2024 · 6 comments
Open

feat: add --set-exit-if-changed on pub upgrade #4146

alestiago opened this issue Feb 26, 2024 · 6 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@alestiago
Copy link

alestiago commented Feb 26, 2024

Description

As a developer, I would like to have the option "--set-exit-if-changed" when running pub upgrade --tighten.

Semantics

From #4146 (comment):

I would expect --set-exit-if-changed to set the exit code on a change. See the following issue in the dart_style for reference dart-lang/dart_style#365, or the implementation.

For example, if pub upgrade --tighten does not modify any file then the exit code should be 0. Otherwise, if a change was made the exit code should be non-0 (the one being used by dart_style is 1).

Use-case

From #4146 (comment):

The use-case is to be used in continuous integration (CI, for example GitHub workflows). I would like to easily check if a project has all its dependencies tighten up. Having it return a non-0 exit code will fail the workflow and disallow merging the change into the project until the requirement of "all dependencies are tighten up" is fulfilled.

One could parse dry-run to get the desired effect, but I see that as a "hacky" solution, since checking if the output of dry-run contains a specific string is fragile.

@sigurdm
Copy link
Contributor

sigurdm commented Feb 26, 2024

What are the proposed semantics?

@alestiago
Copy link
Author

alestiago commented Feb 26, 2024

I would expect --set-exit-if-changed to set the exit code on a change. See the following issue in the dart_style for reference dart-lang/dart_style#365, or the implementation.

For example, if pub upgrade --tighten does not modify any file then the exit code should be 0. Otherwise, if a change was made the exit code should be non-0 (the one being used by dart_style is 1).

@sigurdm
Copy link
Contributor

sigurdm commented Feb 26, 2024

What is the use-case?

@alestiago
Copy link
Author

alestiago commented Feb 26, 2024

The use-case is to be used in continuous integration (CI, for example GitHub workflows). I would like to easily check if a project has all its dependencies tighten up. Having it return a non-0 exit code will fail the workflow and disallow merging the change into the project until the requirement of "all dependencies are tighten up" is fulfilled.

One could parse dry-run to get the desired effect, but I see that as a "hacky" solution, since checking if the output of dry-run contains a specific string is fragile.

@sigurdm sigurdm added the type-enhancement A request for a change that isn't a bug label Feb 26, 2024
@sigurdm
Copy link
Contributor

sigurdm commented Feb 26, 2024

Perhaps this situation would be detectable in CI with git diff pubspec.yaml after dart pub upgrade --tighten

I'm not sure that is desirable in general to always have tightened dependencies... We would need to also have a way of opting out.

@alestiago
Copy link
Author

alestiago commented Feb 26, 2024

Perhaps this situation would be detectable in CI with git diff pubspec.yaml after dart pub upgrade --tighten

That would also be a solution that avoids parsing the dry-run, thanks for sharing 🙌 . Although, I think it has some minor caveats:

  • It assumes that the name of the file pubspec.yaml -- (this is bearable)
  • It assumes that the only file that would only ever change is the pubspec.yaml -- (this is bearable)
  • Requires the git executable to be in the machine -- (possibly bearable)
  • If any CI steps before alters the pubspec.yaml it would require to commit the previous differences before ensuring tight dependencies. Note that some packages in the ecosystem use the pubspec.yaml to define/configure the package behaviour -- (possible bearable by isolating the tightening step if possible)
  • It would not be consistent with other commands the dartdev package has (where pub is being used), such as dart format.

Overall, I think adding --set-exit-if-changed would improve the developer experience.

I'm not sure that is desirable in general to always have tightened dependencies... We would need to also have a way of opting out.

I'm not sure if I'm understanding this concern correctly, I believe when the option --set-exit-if-changed is not specified the tool would have the same behaviour as the one we have today (opting out and always exiting with 0). It would be exactly the same as with dart_style.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

2 participants