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

add workflow to fetch spdx licenses #62

Merged
merged 3 commits into from
Jun 5, 2024
Merged

add workflow to fetch spdx licenses #62

merged 3 commits into from
Jun 5, 2024

Conversation

elrayle
Copy link
Collaborator

@elrayle elrayle commented May 13, 2024

NOTE: This is marked draft only because the trigger on push to the PR branch needs to be removed. Please do review. I will remove the trigger tomorrow after approved and before merging.


Description

Prior to this, the process for updating licenses and exceptions had to be run manually. This sets up a GitHub Action that will run on a schedule and create a PR in the auto-update-licenses branch. The PR can be merged into main, but should not be deleted.

New Process

  • nightly cron executes the fetch-licenses workflow
  • if there are changes to any of the official spdx license files, they will replace the current local copy of these files
  • the licenses are written directly to the file handling its type (i.e. active, deprecated, exceptions) in the main spdxexp package
  • commits the files to the auto-update-licenses branch and creates a PR from that branch to main

Additional Steps and Challenges

  • once this process is running regularly, check that PRs generate automatically, correctly, and can be merged into the main branch
  • updates to range file needs to be performed manually
  • merging the PR is a manually process
  • need to be notified that the PR exists so it can be merged
  • when the PR merges, the auto-update-licenses branch should NOT be deleted
  • need to periodically rebase auto-update-licenses on main

TODO

  • explore if there is a way to generate the range file changes, but it is much more complicated
  • even better would be to revisit the way ranges are used and their layout in the range file to see if there is a simpler and more predictable way to handle ranges
  • explore if there is a way to auto-rebase, or periodically re-create the auto-update-licenses branch

@elrayle elrayle force-pushed the elr/update-licenses branch 2 times, most recently from ea3b48b to 5033dc8 Compare May 13, 2024 21:06
@elrayle elrayle force-pushed the elr/update-licenses branch 2 times, most recently from 266165b to d8182d5 Compare May 17, 2024 19:36
@elrayle elrayle force-pushed the elr/update-licenses branch 2 times, most recently from 6b41210 to bd1f613 Compare June 3, 2024 17:09
@elrayle elrayle marked this pull request as ready for review June 3, 2024 17:39
cmd/license.go Outdated
@@ -43,61 +42,54 @@ func extractLicenseIDs() error {
return err
}

// create two slices of license IDs, one for deprecated and one for not deprecated
// create two slices of license IDs, one for deprecated and one for active
var activeLicenseIDs []string
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Prefer activeLicenseIDs instead of nonDeprecatedLicenseIDs as it is more descriptive.

@elrayle elrayle force-pushed the elr/update-licenses branch 12 times, most recently from ceda506 to 4258c8f Compare June 4, 2024 14:29
Includes trivial debug statement in cmd/main.go.
@elrayle elrayle marked this pull request as draft June 4, 2024 14:37
Copy link
Contributor

@ajhenry ajhenry left a comment

Choose a reason for hiding this comment

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

Looks like a great tool to add!

with:
token: ${{ secrets.GITHUB_TOKEN }}
commit-message: Add updated license files
branch: auto-update-licenses
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it possible to append a unique hash to the name of the branch to avoid collisions and problems with the branch? Something like this

Suggested change
branch: auto-update-licenses
branch: auto-update-licenses-${{ github.run_id }}-${{ github.run_attempt }}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

peter-evans/create-pull-request will either create the branch or update it and associated PR if it already exists. As this is designed to run once a day on a schedule, the chances that there will be a race condition seem unlikely. I like that the PR will be updated as it prevents proliferation of PRs if one is not immediately merged.

I'd like to keep it as is for now. If in reality there are collisions or other problems, we can add a unique branch identifier at that time.

required: false
default: 'false'
schedule:
- cron: '0 0 * * *' # Runs at midnight ET
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- cron: '0 0 * * *' # Runs at midnight ET
- cron: '0 0 * * *' # Runs at midnight UTC

Cron is UTC 😄

run: |
cd cmd
echo "Current branch: $(git branch)"
go run . extract -l -e
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to setup go for this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Actions have go available without setup. The version of go isn't terribly important to be able to run this simple script.

@elrayle elrayle marked this pull request as ready for review June 5, 2024 18:32
@elrayle elrayle merged commit f191b12 into main Jun 5, 2024
2 checks passed
@elrayle elrayle deleted the elr/update-licenses branch June 5, 2024 18:32
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