ckeditor / ckeditor4-workflows-common

Shared CKEditor 4 GitHub workflows.
1 stars 2 forks source link

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

Open f1ames opened 3 years ago

f1ames commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

It is blocked by #4 for now.

f1ames commented 3 years ago

Blocked by #14.

f1ames commented 3 years ago

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 commented 3 years ago

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

f1ames commented 3 years ago

There is already a PR in progress covering that issue, see https://github.com/ckeditor/ckeditor4-workflows-common/pull/28.

sculpt0r commented 3 years ago

ā„ļø Let's wait with this issue - until we decide if the current bot-updates flow is really unpleasant.