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

Fail on --vcs-* options #3241

Merged
merged 1 commit into from
Jan 22, 2024
Merged

Fail on --vcs-* options #3241

merged 1 commit into from
Jan 22, 2024

Conversation

fthomas
Copy link
Member

@fthomas fthomas commented Dec 10, 2023

This makes --vcs-* options unusable. If they are used, Scala Steward will print a message and exit with an error.

These options are deprecated since #2916 and should be replaced with the corresponding --forge-* options.

This makes `--vcs-*` options unusable. If they are used, Scala Steward
will print a message and exit with an error.

These options are deprecated since #2916
and should be replaced with the corresponding `--forge-*` options.
Copy link

codecov bot commented Dec 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b6162bc) 91.06% compared to head (b7ef418) 91.07%.
Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3241   +/-   ##
=======================================
  Coverage   91.06%   91.07%           
=======================================
  Files         163      163           
  Lines        3391     3394    +3     
  Branches      309      300    -9     
=======================================
+ Hits         3088     3091    +3     
  Misses        303      303           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@fthomas
Copy link
Member Author

fthomas commented Dec 10, 2023

Heads-up @tgodzik: Is the public Scala Steward instance already using the --forge-* options instead of --vcs-*? If not, it will stop working once this PR is merged.

Copy link
Contributor

@exoego exoego left a comment

Choose a reason for hiding this comment

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

To not surprise users in winter holiday, let's merge this in 2024 January or later.

@fthomas
Copy link
Member Author

fthomas commented Dec 11, 2023

Sounds good to me.

@tgodzik
Copy link
Contributor

tgodzik commented Dec 11, 2023

Heads-up @tgodzik: Is the public Scala Steward instance already using the --forge-* options instead of --vcs-*? If not, it will stop working once this PR is merged.

Thanks for the heads up! I updated it and deployed the new config.

@fthomas fthomas added the enhancement New feature or request label Dec 20, 2023
@fthomas
Copy link
Member Author

fthomas commented Jan 9, 2024

My plan is to merge this on 2024-01-22 and maybe also release 0.29.0 then.

@fthomas fthomas merged commit 33d3ad7 into main Jan 22, 2024
10 checks passed
@fthomas fthomas deleted the topic/fail-on-vcs-options branch January 22, 2024 18:47
fthomas added a commit that referenced this pull request Jan 22, 2024
Successor to #3241. This should not be merged before Q3 2024 so that
people had enough time to see the current deprecation messages and
migrate to the `--forge-*` options.
@fthomas fthomas mentioned this pull request Jan 22, 2024
@rtyley
Copy link
Contributor

rtyley commented Jan 23, 2024

Just to note that the minimum necessary version of scala-steward-action (if you're using it) after this PR is v2.62.0 - otherwise the run will fail, and you'll see error messages like these:

Error: --vcs-api-host is deprecated; use --forge-api-host instead
Error: --vcs-login is deprecated; use --forge-login instead

image

Curiously, although I can see that #2916 introduced deprecation of the old CLI options back in January 2023, along with updates to the Markdown doc, I haven't been seeing the deprecation warnings appear in the actual Scala Steward runs themselves, eg this recent run doesn't contain the string 'deprecated in favor of'.

@fthomas
Copy link
Member Author

fthomas commented Jan 23, 2024

I haven't been seeing the deprecation warnings appear in the actual Scala Steward runs themselves, eg this recent run doesn't contain the string 'deprecated in favor of'.

That string was only visible in the help message of the command-line options. Scala Steward did not log a warning if that option was used after #2916. It would have been nice if it had done that.

@rtyley
Copy link
Contributor

rtyley commented Jan 23, 2024

Ah ok! No worries!

@mzuehlke mzuehlke added this to the 0.29.0 milestone Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants