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.

Sign up for GitHub

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

Jump to bottom

Unset multiple commands at once #147

Merged
merged 9 commits into from
Sep 27, 2019
Merged

Unset multiple commands at once #147

merged 9 commits into from
Sep 27, 2019

Conversation

boonwj
Copy link
Contributor

@boonwj boonwj commented Sep 26, 2019 •
edited by gopinath-langote
Loading

Description

This PR implements the functionality to unset multiple commands in a single call.

Any failures encountered during the process will be printed but does not stop the operation.

Fixes/Implements: # (issue)

#141

New dependencies introduced?

N.A.

Checklist:

  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog draft for corresponding release
  • I have update README (If needed)

@welcome
Copy link

welcome bot commented Sep 26, 2019

🙏 Thanks for opening this pull request! Please check out our contributing guidelines.

@boonwj
Add feature to unset multiple commands at once
735f4ac 
Allow multiple command names to be passed to unset.

Each command name will be processed in sequence, if not found, a failure
to remove message will be printed and the action will continue.

ref: gopinath-langote#141
@boonwj boonwj force-pushed the feat/unset_multiple_commands branch from 73a17d0 to 735f4ac Compare September 26, 2019 16:10
@gopinath-langote gopinath-langote added enhancement New feature or request Hacktoberfest Support to https://hacktoberfest.digitalocean.com/ In Progress Someone working on the issue labels Sep 26, 2019
@gopinath-langote gopinath-langote added this to the v1.4.0 milestone Sep 26, 2019
boonwj and others added 4 commits September 26, 2019 22:20
@boonwj
Add test case with remaining commands after unset
cbf8d23
@boonwj
Change error message format for unset command
779a68c 
- Error message format changed to red, bold and single line
@boonwj
Add check for config rewrite after unset command
d49a416 
Configuration should be rewritten only when there are changes
after the unset command.
@boonwj
Merge branch 'master' into feat/unset_multiple_commands
12f0f43
@boonwj
Copy link
Contributor Author

boonwj commented Sep 26, 2019

Made the following changes based on feedback

  • Add additional test to cover identified edge case
  • Change error message format
  • Update previous test case based on new error message format
  • Add check to ensure that config is only rewritten when there are changes after the unset operation

@gopinath-langote
Copy link
Owner

LGTM so far.

Could you please also - Change readme in unset section to address one/more commands to unset and it's behavior.

@gopinath-langote
Copy link
Owner

Also,
Change the command short/long description.


Usage:
  1build [flags]
  1build [command]

Available Commands:
  init        Create default project configuration
  list        Show all available commands from the current project configuration
  set         Set new or update the existing command in the current project configuration
  unset       Remove the existing command in the current project configuration

Flags:
  -h, --help      help for 1build
      --version   version for 1build

Use "1build [command] --help" for more information about a command.

From
unset Remove the existing command in the current project configuration

to
unset Remove one or more existing command(s) in the current project configuration

gopinath-langote
gopinath-langote approved these changes Sep 27, 2019
View reviewed changes
Copy link
Owner

@gopinath-langote gopinath-langote left a comment

Choose a reason for hiding this comment

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

LGTM: Thanks for the PR

@gopinath-langote gopinath-langote merged commit a078577 into gopinath-langote:master Sep 27, 2019
@welcome
Copy link

welcome bot commented Sep 27, 2019

Congrats on merging your first pull request! We here at 1build are proud of you! 🙏

@gopinath-langote gopinath-langote changed the title Implements functionality to unset multiple commands in one call Unset multiple commands at once Sep 27, 2019
@boonwj boonwj deleted the feat/unset_multiple_commands branch September 27, 2019 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gopinath-langote gopinath-langote gopinath-langote approved these changes

Assignees

@boonwj boonwj

Labels
enhancement New feature or request Hacktoberfest Support to https://hacktoberfest.digitalocean.com/ In Progress Someone working on the issue
Projects
None yet
Milestone
v1.4.0
Development

Successfully merging this pull request may close these issues.
2 participants
@boonwj @gopinath-langote