Skip to content
This repository was archived by the owner on Mar 11, 2024. It is now read-only.

Support for publishing multiple specs from a directory#8

Merged
tonglil merged 23 commits intonytimes:mainfrom
bajalnyt:upload-from-dir
Sep 20, 2022
Merged

Support for publishing multiple specs from a directory#8
tonglil merged 23 commits intonytimes:mainfrom
bajalnyt:upload-from-dir

Conversation

@bajalnyt
Copy link
Copy Markdown
Contributor

@bajalnyt bajalnyt commented Aug 10, 2022

Adds an [optional] parameter specs_dir to the step. If provided, the code scans through all files in the given directory and pushes them one by one.

We have a monorepo that has several yamls, and it is tedious to add a step each time a new spec is created. See here

Need help verifying, is it possible to push the image with a tag, so that I can try it out from our repo?

Closes #5.

Comment thread README.md Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown
Member

@tonglil tonglil left a comment

Choose a reason for hiding this comment

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

Thanks for submitting!

While I don't particularly like overwriting vargs.Spec for each file, it's acceptable.

Other than the comments, can you add some usage/docs in the README?

We probably need to setup some CI for this as well.
ATM this project is only maintained with volunteered time.

Comment thread main.go Outdated
Comment thread main.go
Comment thread main.go Outdated
Comment thread main.go Outdated
Comment thread main.go Outdated
Comment thread main.go
@bajalnyt
Copy link
Copy Markdown
Contributor Author

bajalnyt commented Aug 20, 2022

@tonglil , thanks a lot for the review!

I have made those changes, and added a unit test for the code in main.go. The integration test won't run without the key/credential passed in, but it can be used to verify that the specs are picked up correctly.
I mocked it out and converted it into a unit test.

Happy to help build out a CI for this repo, if I you can activate this repo in drone (I don't have the needed access)

@bajalnyt
Copy link
Copy Markdown
Contributor Author

@tonglil : appreciate a re-review when you get a chance. Also happy to QA this in one of our repos, if this plugin can be pushed with a specific tag before an official release. Thanks!

Copy link
Copy Markdown
Member

@tonglil tonglil left a comment

Choose a reason for hiding this comment

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

Thanks for all the hard work @bajalnyt.

I'm creating the CI now. Can you take a look at and rebase on-top of #9?

Thanks!

Comment thread .gitignore Outdated
@tonglil tonglil requested a review from a team as a code owner September 19, 2022 22:33
Comment thread .gitignore
@tonglil tonglil merged commit d02e9c5 into nytimes:main Sep 20, 2022
@bajalnyt bajalnyt deleted the upload-from-dir branch September 20, 2022 20:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants