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

Rework git elegant configure command #132

Merged
merged 1 commit into from Jun 6, 2019
Merged

Rework git elegant configure command #132

merged 1 commit into from Jun 6, 2019

Conversation

alexbeatnik
Copy link
Contributor

Issue #130 implemented   
1. git elegant configure renamed to configure-repository
2. command git configure-repository works like git elegant configure --local
3. Tests and README updated

Copy link
Contributor

@extsoft extsoft left a comment

Choose a reason for hiding this comment

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

@alexbeatnik also, you didn't update completions (

configure|econfigure)
). I believe we don't need them anymore.

@@ -23,8 +23,7 @@ _alias_message="add git aliases for all 'elegant git' commands"


_configure() {
MODE=$1; shift
FUNCTIONS=$@
FUNCTIONS=$@
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
FUNCTIONS=$@
FUNCTIONS=$@

--local) _configure --local ${LOCALS[@]} ;;
*) exit 11 # @todo #9 Add usage message if mandatory argument is not set for `git-elegant configure`.
esac

Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbeatnik please review empty lines. They don't give any value.

clean-git
}

@test "'configure': '--local' option is available" {
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbeatnik this option is unavailable. WHy do we have this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@extsoft Thanks for review, fixed

Copy link
Contributor

@extsoft extsoft left a comment

Choose a reason for hiding this comment

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

@alexbeatnik and please squash all commits into one.

@@ -18,8 +18,8 @@ _git_elegant() {
push|epush)
COMPREPLY=( $(compgen -W "$(git branch | grep \* | cut -d ' ' -f2)" ${cur}) )
return 0 ;;
configure|econfigure)
COMPREPLY=($(compgen -W '--global --local' -- ${cur}) )
configure-repository|econfigure-repository)
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbeatnik we don't need this completion as it's empty. Please remove.

esac
}
_configure _user_name _user_email _core_comment_char _apply_whitespace
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@alexbeatnik please add an empty line to the end of the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@extsoft Fixed

Copy link
Contributor

@extsoft extsoft left a comment

Choose a reason for hiding this comment

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

@alexbeatnik could you please search for usage of this command inside the project and replace with a new one? Like

git elegant configure --local

Issue #130 implemented
1. git elegant configure renamed to configure-repository
2. command git configure-repository works like git elegant configure --local
3. Tests and README updated
@extsoft extsoft self-requested a review June 6, 2019 14:18
@extsoft
Copy link
Contributor

extsoft commented Jun 6, 2019

@rultor please merge.

@rultor
Copy link

rultor commented Jun 6, 2019

@rultor please merge.

@extsoft OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit deaefad into bees-hive:master Jun 6, 2019
@rultor
Copy link

rultor commented Jun 6, 2019

@rultor please merge.

@extsoft Done! FYI, the full log is here (took me 2min)

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.

None yet

4 participants