Skip to content

Conversation

@jasp00
Copy link
Member

@jasp00 jasp00 commented Feb 16, 2017

Drop pipe in contributors task

Pipes hide some errors.

Drop pipe in contributors task
@jasp00 jasp00 mentioned this pull request Feb 16, 2017
@jasp00
Copy link
Member Author

jasp00 commented Feb 16, 2017

This will only stop erasing CONTRIBUTORS because of errors. It looks like Git is failing (LMMS/lmms@c0ce75e, LMMS/lmms@d0ef311). Any error output?

@jasp00
Copy link
Member Author

jasp00 commented Feb 16, 2017

When run from a cron job, git shortlog does not work. I am investigating.

@jasp00
Copy link
Member Author

jasp00 commented Feb 16, 2017

Fixed.

Copy link
Member

@tresf tresf left a comment

Choose a reason for hiding this comment

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

Can we use the -pipefail flag then?

set -e

git shortlog -sne | cut -c 8- > doc/CONTRIBUTORS
git shortlog -sne > tmp/shortlog
Copy link
Member

Choose a reason for hiding this comment

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

can you just $var it and avoid the unnecessary mkdir/stdout/rm logic?

Copy link
Member

Choose a reason for hiding this comment

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

I tested -o pipefail and it does exactly what you're achieving without the temporary file. Merged in e326760.

@tresf
Copy link
Member

tresf commented Feb 19, 2017

Superseded by e326760

@tresf tresf closed this Feb 19, 2017
@jasp00 jasp00 deleted the task-contributors branch March 16, 2017 22:52
@jasp00
Copy link
Member Author

jasp00 commented Mar 16, 2017

pipefail is a bashism, #!/bin/sh should be replaced with #!/bin/bash. AFAIK, the POSIX way is to use temporary files.

@tresf
Copy link
Member

tresf commented Mar 16, 2017

Bash is our defacto standard, that's fine.

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