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

Fix golangci and update versions #76

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

Conversation

shibumi
Copy link
Collaborator

@shibumi shibumi commented Nov 29, 2024

Our Github actions are heavily outdated. Let's update them to newer versions.

@shibumi shibumi force-pushed the fix-ci branch 2 times, most recently from 3bacf5c to 16884d7 Compare November 29, 2024 12:01
@shibumi
Copy link
Collaborator Author

shibumi commented Nov 29, 2024

package-url/purl-spec@c5681ae introduced breakage for our Go implementation or it's something we have to fix in this project to comply with the spec.

@shibumi shibumi force-pushed the fix-ci branch 3 times, most recently from f9a05eb to fbad0e2 Compare November 29, 2024 12:07
package-url/purl-spec@c5681ae broke
our tests. Let's pin the test file for now
to unblock incoming PRs. We definitely have to
fix the drift to the upstream spec.
@shibumi
Copy link
Collaborator Author

shibumi commented Nov 29, 2024

@sschuberth Can you approve this PR? It fixes the broken golangci and unblocks: #73

Also, I had to pin the test suite file. Looks like the upstream spec changed OR the packageurl-go version has a spec-drift.
We shoudl fix this ASAP and maybe create a trigger that automatically tests the go suite when a new commit hits the spec repository (either time or event based.. I have some ideas there).

@sschuberth
Copy link
Member

sschuberth commented Nov 29, 2024

@sschuberth Can you approve this PR? It fixes the broken golangci and unblocks: #73

Sorry @shibumi, I was asked by @pombredanne to not do any "drive by" reviews anymore. As I'm unsure whether that only applies to the purl-spec project itself, or also to projects for implementations like this, I'll refrain from a review / approval until this is clarified.

@shibumi
Copy link
Collaborator Author

shibumi commented Nov 29, 2024

@sschuberth no problem :)

@pombredanne do you know someone else who would be willing to maintain this project together with me? I need someone who can review PRs

@shibumi
Copy link
Collaborator Author

shibumi commented Dec 8, 2024

@sschuberth I am sure that @pombredanne only meant the specification repository, because that one is VERY important (lot's of projects are depending on it and it's our source of truth).

I really see no problem in approving this PR, here :)

Especially, because @pombredanne seems to be very inactive :/

@stevespringett, @johnmhoran, @iamwillbar, @jkowalleck, sorry for CC'ing you all. Can maybe one of you approve this PR here or can you appoint a second owner for this repository? Currently, all PRs are blocked and we should really fix these CI bugs ASAP.

If you need a good candidate, I can maybe find someone from the open source software supply chain security community.

- name: Download test data
run: curl -L https://raw.githubusercontent.com/package-url/purl-spec/master/test-suite-data.json -o testdata/test-suite-data.json
run: curl -L https://raw.githubusercontent.com/package-url/purl-spec/0dd92f26f8bb11956ffdf5e8acfcee71e8560407/test-suite-data.json -o testdata/test-suite-data.json
Copy link
Member

Choose a reason for hiding this comment

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

I've taken a look again, and this PR does too many unrelated things in a single commit, IMO. This line is the only one that matches the topic of the commit message AFAIU. It should probably also be annotated with a # TODO: ... statement to make the work-around visible without running git blame.

All other version updates from this commit should IMO go to another commit in the same PR to clearly separate the intentions.

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.

2 participants