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

Manual updates 20240913 Binderator bumping fixed #963

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

moljac
Copy link
Contributor

@moljac moljac commented Sep 13, 2024

Currently Binderator bumps even dependencyOnly = true which is incorrect resulting in NugetVersions which do not exist and resulting in following config file

config.json

This PR fixes this behavior and fetches latest version from nuget.org.

config.json

Added Console output too.

@moljac moljac requested a review from jpobst September 13, 2024 14:07
@jpobst
Copy link
Contributor

jpobst commented Sep 13, 2024

You're correct that we should not be blindly bumping these. In fact, I don't think there's any reason to change these now, as they are all intended to be fixed versions. If we update them to the latest version from NuGet then binderator will report that it cannot find the fixed versions that it needs and we'll have to revert those bumps as well.

I think we just need to have the tooling ignore them, and we change them manually on the rare occasion they need to change:
#964

@moljac
Copy link
Contributor Author

moljac commented Sep 14, 2024

I don't think there's any reason to change these now, as they are all intended to be fixed versions.

Exact/pinned version dependencies are rare. I did manually update dependencies every 3-4 updates, because it was manual task and cumbersome.

If we update them to the latest version from NuGet then binderator will report that it cannot find the fixed versions that it needs and we'll have to revert those bumps as well.

True for exact/pinned version dependencies which are few. And this is what I did in this PR.
It

  1. bumped bindings
  2. updated dependencies to latest published on nuget
  3. manually reverted exact/pinned versions

I think we just need to have the tooling ignore them, and we change them manually on the rare occasion they need to change:

Yes. I did ignore those, but it can happen that some dependencies are not needed anymore and can be removed.

see:

32b2c11

I did this step of removing all dependencies and let binderator "recalculate" new dependencies twice a year.

Ignoring updating dependencies and thus introducing older versions of dependencies could also cause

#764

Now that we have everything in one binderator config file, only exact/pinned version dependencies will remain as dependencies, so updating dependencies problem is solved partially by merging to configs.

@jpobst
Copy link
Contributor

jpobst commented Sep 14, 2024

I'm not entirely following what you are advocating here:

Exact/pinned version dependencies are rare.

True for exact/pinned version dependencies which are few.

Now that we have everything in one binderator config file, only exact/pinned version dependencies will remain as dependencies

We only have exact/pinned version dependencies left in config.json, so if we update them to the latest version on NuGet we will always have to revert them. There are no dependencies left that should have their version changed automatically.

some dependencies are not needed anymore and can be removed

If this is the problem we want to solve, I think we should have binderator track which dependencies in the config.json it doesn't use to build the dependency graph. It could then report them as unneeded. However, we only have 6 dependencies left in config.json, so it may not be worth writing the code to do that.

@jpobst jpobst removed their request for review November 15, 2024 15:54
@moljac
Copy link
Contributor Author

moljac commented Dec 17, 2024

/asp run

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