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

Should update-deps workflow respect packages semver from package.json? #10

Open
f1ames opened this issue Dec 7, 2020 · 8 comments
Open
Labels
size:S The issue should take between 4 hours to 2 days to finish. status:confirmed An issue confirmed by the development team. status:pending More information is needed to proceed with the issue. type:feature A feature request.

Comments

@f1ames
Copy link
Contributor

f1ames commented Dec 7, 2020

We have already discussed it a little when working on the initial script. The difference between native npm update and npm-check is that the former one respects semver while the later one goes all the way to the latest deps version.

Generally, we would like to have a possibility to update to latest versions, but OTOH we have some cases when some packages cannot be yet updated which makes update-deps scripts useless for some time periods. The ideal approach would be possibility to update deps to latest major by default unless it is not specified otherwise.

The cases we already encountered:

And here we have few ideas:

Leave as is

We can just live with this for a while and see if it gets more problematic with time or is acceptable and try to come up with reasonable workflow for breaking deps updates.

npm update

Keeping deps versions as intended is what npm update does and what package.json is for. So we could use npm update but loosen semver (to allow major updates) for all/most of the deps.

The downside is, anyone starting project/package development running npm i may end up with some latest major package (which is still not updated on remote and not tested) which breaks the project. This may be quite confusing.

npm-check with additional config

We could do this and create additional config to mark which deps shouldn't be bumped but it really duplicates the purpose of package.json.


To sum up, from my perspective it will be good to test the current approach for some time still and see how it works. It's still more efficient than single dependabot deps update PRs. Also automatic major deps updates somehow forces us to update deps regularly without staying on fixed version which gets outdated with time.

@f1ames f1ames added the type:feature A feature request. label Dec 7, 2020
@f1ames
Copy link
Contributor Author

f1ames commented Dec 22, 2020

Fourth proposal:

Ability to define update strategy

I thought it could be useful to have two update workflows - one respecting semver (so probably using npm update) and the other one with npm-check going with latest deps. This could be configurable via config and default to semver compatible updates.

We could then run "minor" updates once a week and "major" ones once a month. This should give us the best of both words - quick and frequent minor updates (which shouldn't break anything) and major updates from time to time to keep deps up-to-date but without spending too much time too often on them.

@jacekbogdanski
Copy link
Member

jacekbogdanski commented Dec 22, 2020

To sum up, from my perspective it will be good to test the current approach for some time still and see how it works. It's still more efficient than single dependabot deps update PRs. Also automatic major deps updates somehow forces us to update deps regularly without staying on fixed version which gets outdated with time.

👍🏻

Ability to define update strategy

I would go with that if we find out that we still face problems with the current approach.

@f1ames f1ames added status:confirmed An issue confirmed by the development team. size:S The issue should take between 4 hours to 2 days to finish. labels Jan 4, 2021
@f1ames
Copy link
Contributor Author

f1ames commented Jan 18, 2021

Ok, let's go with Ability to define update strategy as it seems to be the most reasonable and flexible approach 👍

It is blocked by #4 for now.

@f1ames f1ames added the status:blocked An issue which development is blocked by another issue (internal or external one). label Jan 18, 2021
@f1ames f1ames removed the status:blocked An issue which development is blocked by another issue (internal or external one). label Feb 26, 2021
@f1ames f1ames added the status:blocked An issue which development is blocked by another issue (internal or external one). label Apr 9, 2021
@f1ames
Copy link
Contributor Author

f1ames commented Apr 9, 2021

Blocked by #14.

@f1ames
Copy link
Contributor Author

f1ames commented Apr 27, 2021

When modifying any workflow it will be good to provide full tests coverage so we should cover #21 too when working on this issue.

@Dumluregn
Copy link
Contributor

Looks like this issue is no longer blocked since #14 was fixed.

@Dumluregn Dumluregn self-assigned this May 7, 2021
@Dumluregn Dumluregn removed the status:blocked An issue which development is blocked by another issue (internal or external one). label May 7, 2021
@f1ames
Copy link
Contributor Author

f1ames commented Jul 29, 2021

There is already a PR in progress covering that issue, see #28.

@sculpt0r sculpt0r self-assigned this Jul 29, 2021
@sculpt0r sculpt0r added the status:pending More information is needed to proceed with the issue. label Nov 2, 2021
@sculpt0r
Copy link
Contributor

sculpt0r commented Nov 2, 2021

❄️ Let's wait with this issue - until we decide if the current bot-updates flow is really unpleasant.

@sculpt0r sculpt0r removed their assignment Nov 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:S The issue should take between 4 hours to 2 days to finish. status:confirmed An issue confirmed by the development team. status:pending More information is needed to proceed with the issue. type:feature A feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants