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.

Sign up for GitHub

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

Jump to bottom

SPECS/cni-plugins: update to v1.3.0 and set version while building #6396

Merged
merged 1 commit into from
Oct 14, 2023
Merged

SPECS/cni-plugins: update to v1.3.0 and set version while building #6396

merged 1 commit into from
Oct 14, 2023

Conversation

invidian
Copy link
Contributor

@invidian invidian commented Oct 11, 2023 •
edited
Loading

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • Packages depending on static components modified in this PR (Golang, *-static subpackages, etc.) have had their Release tag incremented.
  • Package tests (%check section) have been verified with RUN_CHECK=y for existing SPEC files, or added to new SPEC files
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json, .github/workflows/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/data/licenses.json, ./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge
Summary

What does the PR accomplish, why was it needed?

To address issues stated in #6339

Change Log
  • cni-plugins has been updated to latest available version v1.3.0 and will now correctly print installed version when binaries are run with -v
Does this affect the toolchain?

YES/NO
NO

Associated issues
Test Methodology
  • None

@invidian invidian requested a review from a team as a code owner October 11, 2023 09:35
@microsoft-github-policy-service microsoft-github-policy-service bot added the main PR Destined for main label Oct 11, 2023
@invidian invidian mentioned this pull request Oct 11, 2023
Closed
@invidian invidian force-pushed the update-cni-plugins branch 3 times, most recently from 55383ee to 5c54ee3 Compare October 11, 2023 13:02
@invidian
Copy link
Contributor Author

I finally managed to build package locally. Pushed a fix for setting the version. It builds and prints correctly now.

@christopherco christopherco requested a review from a team October 12, 2023 02:00
@christopherco
Copy link
Contributor

Buddy Build Queued: 434883

@christopherco
Copy link
Contributor

Buddy Build Queued: 434883

Build passed

@invidian
Copy link
Contributor Author

Spec linting is failing because of other packages. Should I fix those as part of this PR?

christopherco
christopherco requested changes Oct 12, 2023
View reviewed changes
Copy link
Contributor

@christopherco christopherco left a comment

Choose a reason for hiding this comment

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

Changes themselves look good but there is a merge conflict to resolve in the spec changelog

SPECS/cni-plugins/cni-plugins.spec Outdated Show resolved Hide resolved
@christopherco
Copy link
Contributor

Spec linting is failing because of other packages. Should I fix those as part of this PR?

No, this PR can just focus on the cni-plugins change

@invidian
SPECS/cni-plugins: update to v1.3.0 and set version while building
3f36671 
Refs microsoft#6339

Also move declarations around to satisfy linter.

Signed-off-by: Mateusz Gozdek <mgozdek@microsoft.com>
@christopherco christopherco merged commit f5ee784 into microsoft:main Oct 14, 2023
10 checks passed
@invidian invidian deleted the update-cni-plugins branch October 14, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfrw mfrw mfrw left review comments

@Camelron Camelron Camelron approved these changes

@christopherco christopherco christopherco approved these changes

@romoh romoh romoh approved these changes

Assignees
No one assigned
Labels
main PR Destined for main
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@invidian @christopherco @mfrw @romoh @Camelron