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

Mr show config comment #417

Merged
merged 2 commits into from
Sep 6, 2020
Merged

Conversation

prarit
Copy link
Collaborator

@prarit prarit commented Sep 6, 2020

Add a local "comments" config option that always sets "--comments" for the mr and issue show commands.

Call the config setup code from the main Run function.  This allows
config variables to be used for all functions.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
@codecov
Copy link

codecov bot commented Sep 6, 2020

Codecov Report

Merging #417 into master will increase coverage by 0.12%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #417      +/-   ##
==========================================
+ Coverage   62.17%   62.30%   +0.12%     
==========================================
  Files          50       50              
  Lines        3141     3146       +5     
==========================================
+ Hits         1953     1960       +7     
+ Misses       1034     1032       -2     
  Partials      154      154              
Impacted Files Coverage Δ
cmd/mr_show.go 81.37% <85.71%> (+0.26%) ⬆️
cmd/issue_show.go 93.75% <100.00%> (+1.75%) ⬆️

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 084623b...4104b40. Read the comment docs.

I'd like to always display the full comments when viewing an MR or Issue.
Add a "comments" config variable that sets the "--comments" flag on
'mr show' and 'issue show'.

Add a "comments" local config option for the show commmands.

Additional fix: Fix a leftover HCL to TOML change in the README

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
@prarit prarit force-pushed the mr_show_config_comment branch from 14a5073 to 4104b40 Compare September 6, 2020 18:12
@prarit prarit merged commit 146904c into zaquestion:master Sep 6, 2020
viper.SetConfigName("show_metadata")
viper.SetConfigType("toml")
// write data
if _, ok := viper.ReadInConfig().(viper.ConfigFileNotFoundError); ok {
Copy link
Owner

Choose a reason for hiding this comment

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

Might be good to abstract some of this config handling into the config package, perhaps some general purpose functionality for command specific config. It also seems a tad strange that the config overrides (like comments = true) can only be set in the repo specific config?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's funny that you bring this up. When I was writing the code I was thinking the exact same thing: this code could all be centralized into the config code but wasn't 100% sure if you wanted it there. I'll definitely do that in another merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might be good to abstract some of this config handling into the config package, perhaps some general purpose functionality for command specific config. It also seems a tad strange that the config overrides (like comments = true) can only be set in the repo specific config?

I moved this code over into the config in #430. As for the config overrides :) I'm working on getting the overrides working. I'm pretty close and hopefully should have something in the next few days for your review.

prarit added a commit that referenced this pull request Sep 14, 2020
config: Add WorkTree functions

As suggested in #417 (comment), move this code into the config code.
@zaquestion zaquestion added this to the 0.18.0 milestone Sep 17, 2020
@prarit prarit deleted the mr_show_config_comment branch January 21, 2021 19:30
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