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

(googlechrome) Change the api-url to use the stable version #2421

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

ralfbosz
Copy link
Contributor

@ralfbosz ralfbosz commented Feb 1, 2024

The api-url used for Google Chrome is not the Google Chrome version that will be downloaded.

Description

The url is pointing to the extended releases. These are well behind and also not the version that will be downloaded from the url at this time (giving mismatch on hash). Move it to the stable-url which do match the downloaded msi.

Motivation and Context

  • The versions of extend are way behind
  • Hash mismatch when doing fresh install

How Has this Been Tested?

  • Created the package with choco pack and the right hashes.

Screenshot (if appropriate, usually isn't needed):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the Chocolatey Test Environment(https://github.com/chocolatey-community/chocolatey-test-environment/). Note that we don't support the use of any other environment.
  • The changes only affect a single package (not including meta package).

Original Location

  • Original Repository
  • Open Issues Add the different issues underneath, and tick those that are fixed in this PR
    • Issue 1 link
    • Issue 2 Link
  • Include the link to the opened PR that removes the package from the original location
  • The migration guidelines have been followed

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@pauby
Copy link
Member

pauby commented Feb 1, 2024

This was changed only a couple of weeks ago by @rdenny-vdc - can you comment on this?

We can't keep updating this URL, so we need some clarity on what is the right thing to use (I have no horse in this race as I don't use Google Chrome).

@rdenny-vdc
Copy link
Contributor

rdenny-vdc commented Feb 1, 2024

This was changed only a couple of weeks ago by @rdenny-vdc - can you comment on this?

We can't keep updating this URL, so we need some clarity on what is the right thing to use (I have no horse in this race as I don't use Google Chrome).

I 100% agree on this change, and it was something that was spoken about in the Chocolatey package comments here and I suggested this exact change. I support this change. Only thing I wasnt sure of was what would happen to package version numbering.

@ralfbosz
Copy link
Contributor Author

ralfbosz commented Feb 1, 2024

The current url does not match the downloaded msi, no idea where you can download the extended version, but the stable url matches the msi-download. With the current url the version is several version behind.

@pauby
Copy link
Member

pauby commented Feb 1, 2024

I 100% agree on this change, and it was something that was spoken about in the Chocolatey package comments here and I suggested this exact change. I support this change. Only thing I wasnt sure of was what would happen to package version numbering.

I'm confused as why you made the change to the extended release if you support it being on stable?

This doesn't affect this PR, but I am concerned about the time it took to put the PR up, review the change, and now we're making another change. Trying to understand the why.

@rdenny-vdc
Copy link
Contributor

I 100% agree on this change, and it was something that was spoken about in the Chocolatey package comments here and I suggested this exact change. I support this change. Only thing I wasnt sure of was what would happen to package version numbering.

I'm confused as why you made the change to the extended release if you support it being on stable?

This doesn't affect this PR, but I am concerned about the time it took to put the PR up, review the change, and now we're making another change. Trying to understand the why.

I worked on the package to try and get it working again, I didnt know at the time there were two different versions at the time and trusted the information I was given. The full PR conversation is here.

#2396 (review)

This was my first contribution.

@pauby
Copy link
Member

pauby commented Feb 5, 2024

This change looks good to me, and I am happy to merge it. However:

  • I don't know enough to understand the consequences of this change (moving from extended to stable.
  • I don't use Chrome.
  • There is no entry in the CODEOWNERS file.

There are two ways forward:

  • Another maintainer, who has more knowledge, merges it.
  • Somebody I know tells me there are no implications and I will merge it.

This is a very popular package so it's not one I want to 'wing it' on.

@rdenny-vdc
Copy link
Contributor

This change looks good to me, and I am happy to merge it. However:

  • I don't know enough to understand the consequences of this change (moving from extended to stable.
  • I don't use Chrome.
  • There is no entry in the CODEOWNERS file.

There are two ways forward:

  • Another maintainer, who has more knowledge, merges it.
  • Somebody I know tells me there are no implications and I will merge it.

This is a very popular package so it's not one I want to 'wing it' on.

@TheCakeIsNaOH would you be happy to help out?

@pauby pauby merged commit 49c46bb into chocolatey-community:master Feb 7, 2024
1 check passed
@AdmiringWorm AdmiringWorm changed the title Change the api-url to use the stable version (googlechrome) Change the api-url to use the stable version Feb 7, 2024
@pauby
Copy link
Member

pauby commented Feb 7, 2024

Having spoken to the team, everybody thinks this sounds like a reasonable change.

Thanks @ralfbosz for your contribution!

@mems
Copy link

mems commented Feb 20, 2024

#2399 is the same fix as this one, but for googlechromebeta

@mems mems mentioned this pull request Mar 11, 2024
11 tasks
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.

5 participants