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

feat(push): adds build and push functionality into different subcommands #21

Merged
merged 11 commits into from
Jun 7, 2022

Conversation

jpower432
Copy link
Contributor

@jpower432 jpower432 commented May 27, 2022

Closes #6

  • Unit tests added
  • Docs updated

Signed-off-by: Jennifer Power barnabei.jennifer@gmail.com

@jpower432 jpower432 added the enhancement New feature or request label May 27, 2022
@jpower432 jpower432 changed the title [WIP] feat(push): adds build and push functionality into different subcommands feat(push): adds build and push functionality into different subcommands Jun 2, 2022
Closes ortelius#6

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Returning the descriptors allows for easier testing of concrete
implementations of the Client interface and allows for post-processing
on descriptors.

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

Looking good. A few changes

cli/build.go Outdated
# Template content in a directory and push to a registry location
client build <directory> --push --destination localhost:5000/myartifacts:latest
# Template content into a specified output directory.
client build mydirectory myoutputdirectory
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to specify --output flag

cli/build.go Outdated
# Template content in a directory without pushing
client build <directory>
# Template content in a directory
client build mydirectory
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this template the content from the specified directory into the current directory?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a default workspace called "client-workspace" that is created in the current directory if the --ouput flag is not used. The content is taken from the specified directory and templated into that directory or whatever workspace is defined with the --output flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added that info the example text

cli/build.go Outdated
# Template content in a directory without pushing
client build <directory>
# Template content in a directory
client build mydirectory
Copy link
Contributor

Choose a reason for hiding this comment

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

In the output of the command, do we want to mention where the workspace directory is located?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is a log output at the end of the command that gives instructions on how to publish the rendered content with the workspace information. It is also logged at the beginning of the operation, but at the debug level. Do you think that is worth bumping to info?

Copy link
Contributor

Choose a reason for hiding this comment

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

At this point, would like to lower the barrier of entry as much as possible, So recommend adding it at Info level

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

README.md Outdated Show resolved Hide resolved
jpower432 and others added 2 commits June 6, 2022 09:37
Co-authored-by: Andrew Block <andy.block@gmail.com>
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
@jpower432 jpower432 requested a review from sabre1041 June 6, 2022 13:49
Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>
Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@sabre1041 sabre1041 merged commit d3bd414 into ortelius:main Jun 7, 2022
jpower432 added a commit to jpower432/client that referenced this pull request Jun 21, 2022
…nds (ortelius#21)

* feat(push): adds build and push functionality into different subcommands

Closes ortelius#6

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* docs: updates README.md with new usage directions for push

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* feat: changes Client interface to allow the descriptors be returned

Returning the descriptors allows for easier testing of concrete
implementations of the Client interface and allows for post-processing
on descriptors.

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* chore: comment clean up in graph and builder packages

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* chore: update README.md with user workflow

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* test: adds artifact digest checking on build unit tests

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* chore: adds logger to push command

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* test(cli): adds logger to push e2e test

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* Update README.md

Co-authored-by: Andrew Block <andy.block@gmail.com>

* docs: adds correct output flags and fixes naming in examples

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

* fix(build): bumps log about workspace to Info level in build command

Signed-off-by: Jennifer Power <barnabei.jennifer@gmail.com>

Co-authored-by: Andrew Block <andy.block@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RFE] Add support for publishing pre-existing artifacts
2 participants