Skip to content

update-license-data: fix latest_tag error#8287

Merged
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
SeekingMeaning:spdx/fix-latest_tag-error
Aug 11, 2020
Merged

update-license-data: fix latest_tag error#8287
MikeMcQuaid merged 1 commit intoHomebrew:masterfrom
SeekingMeaning:spdx/fix-latest_tag-error

Conversation

@SeekingMeaning
Copy link
Copy Markdown
Contributor

Add latest_tag method to utils/spdx

Related: #8215, #8284

cc @issyl0

Add latest_tag method to utils/spdx
@reitermarkus
Copy link
Copy Markdown
Member

Might be better to just return this from download_latest_license_data!.

@Rylan12
Copy link
Copy Markdown
Member

Rylan12 commented Aug 10, 2020

Might be better to just return this from download_latest_license_data!.

I did this in #8260 but I think I like this better. It seems a little arbitrary to have download_latest_license_data! return the tag name.

This way, it's clear what's going on.

@reitermarkus
Copy link
Copy Markdown
Member

It seems a little arbitrary

I don't think it's arbitrary. You download the latest something and as a result you get to know the specific version.

@Rylan12
Copy link
Copy Markdown
Member

Rylan12 commented Aug 10, 2020

You download the latest something and as a result you get to know the specific version.

I think what bothers me is that the download_latest_license_data! method does more than just return the latest tag. It feels like if it should return anything, it should be whether or not it succeeded.

But no strong feelings about it either way.

@MikeMcQuaid MikeMcQuaid merged commit b8f1cf3 into Homebrew:master Aug 11, 2020
@MikeMcQuaid
Copy link
Copy Markdown
Member

Merging as-is as this is a fix. Happy to have another PR refactoring. Thanks @SeekingMeaning!

@SeekingMeaning SeekingMeaning deleted the spdx/fix-latest_tag-error branch August 11, 2020 18:23
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 18, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

outdated PR was locked due to age

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants