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.

Sign up for GitHub

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

Jump to bottom

fix(sbt-plugin): add new default registry URL #29648

Merged
merged 23 commits into from
Jul 1, 2024
Merged

fix(sbt-plugin): add new default registry URL #29648

merged 23 commits into from
Jul 1, 2024

Conversation

george-wilson-rea
Copy link
Contributor

Changes

Change the default repository for SBT plugin lookup to https://repo1.maven.org/maven2/

Context

The bintray service has been sunset and now https://repo.scala-sbt.org/scalasbt/ redirects to a mirror that no longer receives updates. Renovate should just default to maven.org for sbt plugins as it does for sbt packages. See this page for additional context: https://www.scala-sbt.org/1.x/docs/Bintray-For-Plugins.html

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@george-wilson-rea
Replace the default SBT plugin repo URL
b3fe75b 
The bintray service has been sunset and now redirects to a mirror that no longer receives updates,
so we should just default to maven.org as we do with sbt packages.
See this page for additional context: https://www.scala-sbt.org/1.x/docs/Bintray-For-Plugins.html
@george-wilson-rea george-wilson-rea changed the title Replace the default SBT plugin repo URL fix: Replace the default SBT plugin repo URL Jun 13, 2024
@viceice viceice changed the title fix: Replace the default SBT plugin repo URL fix(datasource/sbt-plugin): Replace the default repo URL Jun 13, 2024
@viceice viceice changed the title fix(datasource/sbt-plugin): Replace the default repo URL fix(sbt-plugin): Replace the default repo URL Jun 13, 2024
viceice
viceice requested changes Jun 13, 2024
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

you need to verify that it still works with older plugins.

lib/modules/datasource/sbt-plugin/index.ts Outdated Show resolved Hide resolved
@george-wilson-rea
Merge branch 'main' into update-sbt-plugin-url
c0a0c42
@george-wilson-rea
Add back in the old sbt plugin repo
382e47a 
This repo is needed for plugins from 2021 or older. I've put it after the maven repo, because the maven repo is the default for newer plugins.
@george-wilson-rea
Make sure the plugins repo is in the defaultRegistryUrls
01b45f3
viceice
viceice previously approved these changes Jun 15, 2024
View reviewed changes
@viceice viceice changed the title fix(sbt-plugin): Replace the default repo URL fix(sbt-plugin): add new default registry URL Jun 15, 2024
HonkingGoose
HonkingGoose reviewed Jun 15, 2024
View reviewed changes
Copy link
Collaborator

@HonkingGoose HonkingGoose left a comment

Choose a reason for hiding this comment

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

Did you check if there are readme files related to this change that need to be updated?

rarkins
rarkins requested changes Jun 16, 2024
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Needs at least one test on a "real" repository to make sure it works end to end. This is the type of change which would pass all unit tests while stopping plugin updating working altogether if something were wrong

@george-wilson-rea
Copy link
Contributor Author

@HonkingGoose When I search for "sbt" in the docs directory, I don't see anything that would need updating. Should I be looking anywhere else?

@rarkins How should I test this on a "real" repository? Could you please elaborate?

@rarkins
Copy link
Collaborator

rarkins commented Jun 16, 2024

A public repo on GitHub.com which needs updates affected by this PR.

@george-wilson-rea
Copy link
Contributor Author

Here's a repo: https://github.com/george-wilson-rea/renovate-scala-plugins

In plugins.sbt, sbt-scoverage is set to 2.0.11, which is not the latest version. I expect it to be updated to 2.0.12 but with recommended config, that doesn't happen.

@george-wilson-rea
Copy link
Contributor Author

Here's another repo, with an overridden sbt plugin config. Interestingly, sbt-scoverage is still not getting updated. I'm not sure what's going wrong here.
https://github.com/george-wilson-rea/renovate-scala-plugins-fixed

@george-wilson-rea
Copy link
Contributor Author

Interestingly, it looks like sbt plugins are working by default, but for some reason sbt-scoverage specifically doesn't work with either setup. I might have misdiagnosed the problem here.
How can I investigate what's going wrong with sbt-scoverage?

@HonkingGoose
Copy link
Collaborator

@HonkingGoose When I search for "sbt" in the docs directory, I don't see anything that would need updating. Should I be looking anywhere else?

Hmm, the convention is that each manager/datasource/platform has a readme file. Maybe the sbt-plugin has no readme yet. It should have a readme though!

Please search in a similar directory for examples. 😉

HonkingGoose
HonkingGoose suggested changes Jun 16, 2024
View reviewed changes
Copy link
Collaborator

@HonkingGoose HonkingGoose left a comment

Choose a reason for hiding this comment

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

Please add a readme to both the datasource and manager.

@george-wilson-rea
Copy link
Contributor Author

@HonkingGoose The sbt manager has a readme saying

Extracts Maven artifact dependencies for SBT.

which is accurate. I have added some minimal readmes to the sbt-package and sbt-plugin datasources. Is there anything else I should say in these documents?

@george-wilson-rea
Copy link
Contributor Author

I've merged the latest suggestions from @zharinov, which has unfortunately dismissed the approving review. I've re-requested review per the contribution guidelines. Sorry for the churn.

@rarkins rarkins enabled auto-merge June 26, 2024 06:43
@HonkingGoose HonkingGoose removed the status:blocked Issue is blocked by another issue or external requirement label Jun 26, 2024
HonkingGoose
HonkingGoose suggested changes Jun 26, 2024
View reviewed changes
Copy link
Collaborator

@HonkingGoose HonkingGoose left a comment

Choose a reason for hiding this comment

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

Some docs changes:

  • Follow one sentence per line pattern
  • Make a ordered list of the steps Renovate takes by default
  • Small style fixes

lib/modules/datasource/sbt-package/readme.md Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/readme.md Outdated Show resolved Hide resolved
auto-merge was automatically disabled June 26, 2024 23:07

Head branch was pushed to by a user without write access

@rarkins rarkins enabled auto-merge June 27, 2024 00:26
@george-wilson-rea
Copy link
Contributor Author

Thanks for enabling auto-merge @rarkins . Could you, or someone else, please also approve so that this gets merged? Your last review was dismissed when I merged @zharinov 's suggestions.

viceice
viceice reviewed Jun 28, 2024
View reviewed changes
Copy link
Member

@viceice viceice left a comment

Choose a reason for hiding this comment

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

otherwise LGTM

lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
lib/modules/datasource/sbt-plugin/index.spec.ts Outdated Show resolved Hide resolved
auto-merge was automatically disabled June 28, 2024 13:17

Head branch was pushed to by a user without write access

@rarkins rarkins enabled auto-merge June 28, 2024 13:22
@rarkins rarkins added this pull request to the merge queue Jul 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 1, 2024
@rarkins rarkins added this pull request to the merge queue Jul 1, 2024
Merged via the queue into renovatebot:main with commit 0208d76 Jul 1, 2024
37 checks passed
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 37.421.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@george-wilson-rea george-wilson-rea deleted the update-sbt-plugin-url branch July 1, 2024 23:29
@github-actions github-actions bot mentioned this pull request Jul 4, 2024
Merged
bryannaegele pushed a commit to bryannaegele/renovate that referenced this pull request Jul 8, 2024
@george-wilson-rea @zharinov
@HonkingGoose @viceice @bryannaegele
fix(sbt-plugin): add new default registry URL (renovatebot#29648)
00f695a 
Co-authored-by: Sergei Zharinov <[email protected]>
Co-authored-by: HonkingGoose <[email protected]>
Co-authored-by: Michael Kriese <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@viceice viceice viceice approved these changes

@zharinov zharinov zharinov approved these changes

@rarkins rarkins Awaiting requested review from rarkins

@HonkingGoose HonkingGoose Awaiting requested review from HonkingGoose

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@george-wilson-rea @rarkins @HonkingGoose @zharinov @renovate-release @viceice