ockam icon indicating copy to clipboard operation
ockam copied to clipboard

[WIP] Automate notice file updates (resolves #626)

Open Wryhder opened this issue 10 months ago • 2 comments

Current behavior

The NOTICE is currently not auto-updated (Issue #626).

Proposed changes

This PR modifies and builds on PR #5126. Changes to PR:

  • Modify parseCrates.sh script to use jq
  • Rename notice_update job to match existing format

Additions (WIP):

  • Add automated PR to update NOTICE file
  • ...

Checks

  • [x] All commits in this Pull Request are signed and Verified by Github.
  • [x] All commits in this Pull Request follow the Ockam commit message convention.
  • [x] There are no Merge commits in this Pull Request. Ockam repo maintains a linear commit history. We merge Pull Requests by rebasing them onto the develop branch. Rebasing to the latest develop branch and force pushing to your Pull Request branch is okay.
  • [x] I have read and accept the Ockam Community Code of Conduct.
  • [x] I have read and accepted the Ockam Contributor License Agreement by adding my Git/Github details in a row at the end of the CONTRIBUTORS.csv file in a separate pull request to the build-trust/ockam repository. The easiest way to do this is to edit the CONTRIBUTORS.csv file in the github web UI and create a separate Pull Request, this will mark the commit as verified.

Wryhder avatar Apr 09 '24 16:04 Wryhder

@metaclips Per @mrinalwadhwa's suggested schedule of running the notice file update workflow once a week, should I move the job into its own file, since it seems adding a weekly schedule in all.yml will affect all the other jobs in the file?

Wryhder avatar Apr 09 '24 17:04 Wryhder

suggested schedule

We will be running this check for every PR that is to be merged, this will ensure that contributors themselves update the Notice file if it's outdated, I believe it's fine as is

metaclips avatar Apr 10 '24 19:04 metaclips

@metaclips Thanks for fixing the lint issues in the parseCrates file! I didn't realize I needed to lint it.

I've just updated the NOTICE file. Could you please take a look again and let me know if there's any more changes I need to make?

Wryhder avatar May 02 '24 11:05 Wryhder

Notice CI still fails https://github.com/build-trust/ockam/actions/runs/8923006071/job/24507630076?pr=7872 I think we are missing something

metaclips avatar May 02 '24 13:05 metaclips

Notice CI still fails https://github.com/build-trust/ockam/actions/runs/8923006071/job/24507630076?pr=7872 I think we are missing something

@metaclips Could you please clarify if the workflow is checking out the most current version of the develop branch? I could be wrong but if the workflow generates a new notice file from develop and compares it against the NOTICE file from the PR's last commit, then it seems like as long as there's been crate-related modifications to develop, the PR's going to be playing catch-up until the workflow runs when there are no such changes (after rebasing).

If I'm not completely off and that's what's happening here, what about using something like add-and-commit or replicating such functionality so that the new NOTICE file from the workflow is committed to the PR branch (although I'm not sure yet how to do this)?

Wryhder avatar May 03 '24 14:05 Wryhder

Notice CI still fails https://github.com/build-trust/ockam/actions/runs/8923006071/job/24507630076?pr=7872 I think we are missing something

@metaclips Could you please clarify if the workflow is checking out the most current version of the develop branch? I could be wrong but if the workflow generates a new notice file from develop and compares it against the NOTICE file from the PR's last commit, then it seems like as long as there's been crate-related modifications to develop, the PR's going to be playing catch-up until the workflow runs when there are no such changes (after rebasing).

If I'm not completely off and that's what's happening here, what about using something like add-and-commit or replicating such functionality so that the new NOTICE file from the workflow is committed to the PR branch (although I'm not sure yet how to do this)?

Seems we were using an outdated version of cargo-deny locally. I was able to push an updated version of the Notice file and CI now passes.

metaclips avatar May 03 '24 15:05 metaclips

Seems we were using an outdated version of cargo-deny locally. I was able to push an updated version of the Notice file and CI now passes.

Does this mean cargo-deny should be updated before running the parseCrates script each time? Also, thanks for approving the PR!

Wryhder avatar May 03 '24 15:05 Wryhder

Seems we were using an outdated version of cargo-deny locally. I was able to push an updated version of the Notice file and CI now passes.

Does this mean cargo-deny should be updated before running the parseCrates script each time? Also, thanks for approving the PR!

Yes, we should update the notice file with the latest version of cargo-deny, Nix makes that easy for us.

metaclips avatar May 03 '24 20:05 metaclips