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

Removing example structure from armadactl #2551

Merged
merged 7 commits into from
Jun 21, 2023
Merged

Removing example structure from armadactl #2551

merged 7 commits into from
Jun 21, 2023

Conversation

ShivangShandilya
Copy link
Contributor

@ShivangShandilya ShivangShandilya commented Jun 10, 2023

Special notes for your reviewer:

This PR just removes the example structure from the armadactl

┆Issue is synchronized with this Jira Task by Unito

@codecov
Copy link

codecov bot commented Jun 10, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (f04a35f) 58.57% compared to head (8f6cf0a) 58.57%.

❗ Current head 8f6cf0a differs from pull request most recent head 7a998dc. Consider uploading reports for the commit 7a998dc to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2551   +/-   ##
=======================================
  Coverage   58.57%   58.57%           
=======================================
  Files         236      236           
  Lines       29957    29957           
=======================================
  Hits        17548    17548           
  Misses      11062    11062           
  Partials     1347     1347           
Flag Coverage Δ
armada-server 58.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@@ -17,19 +17,6 @@ func init() {
var rootCmd = &cobra.Command{
Use: "armada-load-tester command",
Short: "Command line utility to submit many jobs to armada",
Long: `
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did we remove the Long description?

Copy link
Contributor Author

@ShivangShandilya ShivangShandilya Jun 12, 2023

Choose a reason for hiding this comment

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

Oops thanks for noticing, I only wanted to remove the example structure, let me correct that

Copy link
Contributor

Choose a reason for hiding this comment

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

And how does this interact with @zuqq PR?

Copy link
Contributor Author

@ShivangShandilya ShivangShandilya Jun 12, 2023

Choose a reason for hiding this comment

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

@zuqq has provided spaces and all in the same example structure that I'm trying to remove, I also commented on his PR but didn't hear back from him. This deletion is not vital to the project its just that if you take a look at any other command-line tool eg: kubectl you won't find any kind of structure, you only see what kubectl does and its commands so only for the purpose to make armadactl look good I have proposed this change

@kannon92 kannon92 enabled auto-merge (squash) June 21, 2023 14:21
@kannon92 kannon92 merged commit 7167c57 into armadaproject:master Jun 21, 2023
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.

2 participants