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

makefile: add rule to download and set swagger and make rule to build the dist #154

Merged
merged 1 commit into from Jul 28, 2021

Conversation

cpanato
Copy link
Member

@cpanato cpanato commented Jul 28, 2021

@cpanato cpanato added this to the 0.2.0 milestone Jul 28, 2021
@bobcallaway
Copy link
Member

looks fine, two thoughts:

  1. should we update README.md (which currently has the developer manually install swagger)?
  2. is there any benefit in having a "dummy" go install of swagger in a GitHub action (not actually used but at least referenced) so that @dependabot knows to flag updates for it?

@cpanato
Copy link
Member Author

cpanato commented Jul 28, 2021

@bobcallaway

  1. already removed the reference in the readme that says to install swagger
  2. the swagger update should not be our decision when need to upgrade? they might have any breaking changes? no strong opinions on that, I can take a look and see if dependabot can act in this case

thanks for the review

@bobcallaway
Copy link
Member

@bobcallaway

  1. already removed the reference in the readme that says to install swagger

https://github.com/sigstore/fulcio/blob/485846c13d586e6e2d213ddb40d7ba05a993530a/README.md#build-for-development

  1. the swagger update should not be our decision when need to upgrade? they might have any breaking changes? no strong opinions on that, I can take a look and see if dependabot can act in this case

They might, but since go-swagger is a big part of our code base I want to make sure we're at least notified when an upgrade is released so we can evaluate it. This can be a separate PR but just wanted to capture the idea

thanks for the review

README.md Show resolved Hide resolved
@cpanato
Copy link
Member Author

cpanato commented Jul 28, 2021

yep, will take a look in the dependabot review and see what we can do, otherwise we can make an action workflow to check that and notify us :)

They might, but since go-swagger is a big part of our code base I want to make sure we're at least notified when an upgrade is released so we can evaluate it. This can be a separate PR but just wanted to capture the idea

@cpanato cpanato changed the title makefile: add rule to download and set swagger and make rule to build the dist WIP makefile: add rule to download and set swagger and make rule to build the dist Jul 28, 2021
@cpanato
Copy link
Member Author

cpanato commented Jul 28, 2021

after discussing with @bobcallaway i will update this PR to use something similar we have in https://github.com/kubernetes-sigs/cluster-api/blob/master/hack/tools/go.mod

@cpanato cpanato requested a review from bobcallaway July 28, 2021 13:13
@cpanato cpanato changed the title WIP makefile: add rule to download and set swagger and make rule to build the dist makefile: add rule to download and set swagger and make rule to build the dist Jul 28, 2021
Copy link
Member

@bobcallaway bobcallaway left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

… the dist

Signed-off-by: Carlos Panato <ctadeu@gmail.com>
@bobcallaway bobcallaway merged commit ea8e5a3 into sigstore:main Jul 28, 2021
@cpanato cpanato deleted the releases-updates branch July 28, 2021 15:43
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.

None yet

2 participants