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

Escape asterisk so that it is surely accepted as a glob #1195

Merged
merged 2 commits into from Jan 8, 2016

Conversation

5 participants
@SanketDG
Member

SanketDG commented Jan 7, 2016

No description provided.

@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Jan 7, 2016

The commit message

  • Commit: 2c361ed268b9961f18ca43f1bca2d39162b1f122
    • Commits must be in the following format: %{scope}: %{description}
  • Commit: c4e4a1c3a43c5af4dca54a27a9a8a7b8e9f2ff04
    • Commits must be in the following format: %{scope}: %{description}

Guidelines are available at http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/#how-to-write-good-commit-messages


This message was auto-generated by https://gitcop.com

GitCop commented Jan 7, 2016

The commit message

  • Commit: 2c361ed268b9961f18ca43f1bca2d39162b1f122
    • Commits must be in the following format: %{scope}: %{description}
  • Commit: c4e4a1c3a43c5af4dca54a27a9a8a7b8e9f2ff04
    • Commits must be in the following format: %{scope}: %{description}

Guidelines are available at http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/#how-to-write-good-commit-messages


This message was auto-generated by https://gitcop.com

@sils sils added the process/wip label Jan 7, 2016

@gitmate-bot gitmate-bot added the size/XS label Jan 7, 2016

@sils

This comment has been minimized.

Show comment
Hide comment
@sils

sils Jan 8, 2016

Member

Hey @SanketDG ! Did you read http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/#how-to-write-good-commit-messages ? I think your shortlog is too long, it definitely needs capitalization after the colon. Rather try something like Escape glob argument then explaining it elaboratively. Explain what you do not what you fix and keep it short and concise. I'll let the second shortlog to you ;)

Second: as github shows - this branch is out of date. You will need to rebase it onto the current master. I recommend you wait with that until we merge #1194 which is in about five minutes, otherwise you'll just have to do it again.

Member

sils commented Jan 8, 2016

Hey @SanketDG ! Did you read http://coala.readthedocs.org/en/latest/Getting_Involved/Writing_Good_Commits/#how-to-write-good-commit-messages ? I think your shortlog is too long, it definitely needs capitalization after the colon. Rather try something like Escape glob argument then explaining it elaboratively. Explain what you do not what you fix and keep it short and concise. I'll let the second shortlog to you ;)

Second: as github shows - this branch is out of date. You will need to rebase it onto the current master. I recommend you wait with that until we merge #1194 which is in about five minutes, otherwise you'll just have to do it again.

SanketDG added some commits Jan 7, 2016

docs: Escape glob argument
In zsh, under default settings, when globs are specified by '=' in
arguments, the asterisk (*) is taken to be a literal asterisk.

So, in here, if the --files argument is --files=src/*.c, it looks
for a file named *.c in src/folder, and wouldn't act as a glob,
i.e. take in account all files in the src/ folder with a '.c'
extension.

So to avoid this, the asterisk is escaped with a leading forward
slash so that it acts as a glob.
docs: Use "=" to specify arguments for consistency
The docs uses "=" to specify the arguments, and for the sake of
consistency, it should be the standard way to specify arguments
in the docs.

Also, a leading forward slash is added to the -f argument so that
it takes a glob for sure, and not a literal asterisk (which
sometimes happens in zsh)
@SanketDG

This comment has been minimized.

Show comment
Hide comment
@SanketDG

SanketDG Jan 8, 2016

Member

@sils1297 Are the commit messages now okay?

Second: as github shows - this branch is out of date. You will need to rebase it onto the current master. I recommend you wait with that until we merge #1194 which is in about five minutes, otherwise you'll just have to do it again.

I did that. Thanks.

Member

SanketDG commented Jan 8, 2016

@sils1297 Are the commit messages now okay?

Second: as github shows - this branch is out of date. You will need to rebase it onto the current master. I recommend you wait with that until we merge #1194 which is in about five minutes, otherwise you'll just have to do it again.

I did that. Thanks.

@sils

This comment has been minimized.

Show comment
Hide comment
@sils

sils Jan 8, 2016

Member

ack, for the sake of a real good commit you would remove the "for consistency" out of the shortlog and mention that in the body (it's a reason, not what you do); because the shortlog is 51 chars (one char about the "soft" maximum) however, we can accept this :) One would also think about correcting the backspace in the other commit so you'd have one backspace correction commit and one for the equal sign. However this commit is rather simple so that's ok too.

Member

sils commented on e34051b Jan 8, 2016

ack, for the sake of a real good commit you would remove the "for consistency" out of the shortlog and mention that in the body (it's a reason, not what you do); because the shortlog is 51 chars (one char about the "soft" maximum) however, we can accept this :) One would also think about correcting the backspace in the other commit so you'd have one backspace correction commit and one for the equal sign. However this commit is rather simple so that's ok too.

@sils

This comment has been minimized.

Show comment
Hide comment
@sils

sils Jan 8, 2016

Member

ack

Member

sils commented on 023fc11 Jan 8, 2016

ack

@sils sils added process/approved and removed process/wip labels Jan 8, 2016

@sils

This comment has been minimized.

Show comment
Hide comment
@sils

sils Jan 8, 2016

Member

@rultor merge

Member

sils commented Jan 8, 2016

@rultor merge

@rultor

This comment has been minimized.

Show comment
Hide comment
@rultor

rultor Jan 8, 2016

Contributor

@rultor merge

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

Contributor

rultor commented Jan 8, 2016

@rultor merge

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

@rultor rultor merged commit e34051b into coala:master Jan 8, 2016

7 checks passed

Scrutinizer 3 updated code elements
Details
ci/circleci Your tests passed on CircleCI!
Details
codecov/project 100.00% remains the same compared to 42de3b1
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
review/gitmate/commit All is well! :)
Details
review/gitmate/manual This commit was acknowledged.
Details
@rultor

This comment has been minimized.

Show comment
Hide comment
@rultor

rultor Jan 8, 2016

Contributor

@rultor merge

@sils1297 Done! FYI, the full log is here (took me 3min)

Contributor

rultor commented Jan 8, 2016

@rultor merge

@sils1297 Done! FYI, the full log is here (took me 3min)

@SanketDG SanketDG deleted the SanketDG:fixdocs branch Jan 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment