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

Added --force-next-version command line param. #1019

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boocmp
Copy link
Contributor

@boocmp boocmp commented Dec 4, 2024

Resolves #1018

Copy link
Collaborator

@mherrmann mherrmann left a comment

Choose a reason for hiding this comment

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

My review was requested but I'm hardly familiar with the code, so feel somebody else should check this work. One thing I'm wondering is why the forcing is necessary. The issue does not explain this at all.

@boocmp
Copy link
Contributor Author

boocmp commented Dec 5, 2024

Gihub has suggested you as a reviewer. I've updated the issue.

Copy link
Collaborator

@antonok-edm antonok-edm left a comment

Choose a reason for hiding this comment

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

IMO passing another argument around like this seems like a bit too much extra complexity for something that we'd do very rarely?

My proposal is, given that right now, the hash is generated using util.generateSHA256HashOfFile...

...let's just make a wrapper around generateSHA256HashOfFile, something like generateVersionedHashOfFile, which can prepend a number to the hashed file, and if we ever need to force an update we can bump the number.

We currently only even calculate contentHash for the adblock packages so this shouldn't require changing much.

@boocmp boocmp force-pushed the version_bumper branch 2 times, most recently from 009cc31 to 83ca30c Compare January 7, 2025 11:21
@boocmp
Copy link
Contributor Author

boocmp commented Jan 7, 2025

Yes, you're right, this may be the first and last time we need it.

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.

A new command line option to force the creation of the next version of the component.
3 participants