Skip to content

mr,issue,snippet: make shorthand flags deprecated#629

Merged
bmeneg merged 1 commit intozaquestion:masterfrom
bmeneg:fix-deprecate-flags
Mar 10, 2021
Merged

mr,issue,snippet: make shorthand flags deprecated#629
bmeneg merged 1 commit intozaquestion:masterfrom
bmeneg:fix-deprecate-flags

Conversation

@bmeneg
Copy link
Copy Markdown
Collaborator

@bmeneg bmeneg commented Mar 9, 2021

Deprecate shorthand flags directly in the commands (mr, issue and snippet).

These shorthands don't really work as expected: the flags from the subcommands are not available for them, so it has been basically a shorthand for the default behavior of a subcommand: lab mr list == lab mr --list, and that's it. IMHO, the subcommand is easier to type than the shorthand, so I don't think it makes sense to keep them around.

All commands with --list, --close and --browse flags, also has the actual
subcommand for each of them. I've seen users getting confused because the
flags in the `list` command can't be used with the shorthand, and tbh,
typing `list` is easier than `--list` (the same is valid for the others
flags).

IMHO the shorthand as a whole should be deprecated, avoiding user confusion
and, possibly, dead code for getting maintained.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
@bmeneg bmeneg requested review from prarit and zaquestion March 9, 2021 14:10
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2021

Codecov Report

Merging #629 (8b27899) into master (78d76dc) will decrease coverage by 1.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #629      +/-   ##
==========================================
- Coverage   57.29%   56.14%   -1.15%     
==========================================
  Files          71       73       +2     
  Lines        4519     4613      +94     
==========================================
+ Hits         2589     2590       +1     
- Misses       1686     1772      +86     
- Partials      244      251       +7     
Impacted Files Coverage Δ
cmd/issue.go 66.66% <100.00%> (+5.55%) ⬆️
cmd/mr.go 66.66% <100.00%> (+5.55%) ⬆️
cmd/snippet.go 76.00% <100.00%> (+3.27%) ⬆️
cmd/milestone_delete.go 66.66% <0.00%> (-33.34%) ⬇️
cmd/milestone_create.go 77.77% <0.00%> (-22.23%) ⬇️
cmd/issue_list.go 63.85% <0.00%> (-9.48%) ⬇️
cmd/issue_create.go 73.68% <0.00%> (-8.67%) ⬇️
cmd/mr_list.go 66.05% <0.00%> (-7.22%) ⬇️
cmd/issue_edit.go 70.53% <0.00%> (-6.92%) ⬇️
cmd/fork.go 56.33% <0.00%> (-5.64%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78d76dc...8b27899. Read the comment docs.

Copy link
Copy Markdown
Collaborator

@prarit prarit left a comment

Choose a reason for hiding this comment

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

LGTM

@bmeneg bmeneg merged commit c79e750 into zaquestion:master Mar 10, 2021
@bmeneg bmeneg mentioned this pull request Mar 10, 2021
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