Skip to content

Add filecheck script to check for essential files #459

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

Closed
wants to merge 1 commit into from

Conversation

tejasbubane
Copy link
Member

Check for presence of description.md and metadata.yml.
Add this script to travis build.

Fixes #457

Check for presence of `description.md` and `metadata.yml`.
Add this script to travis build.

Fixes exercism#457
@Insti
Copy link
Contributor

Insti commented Nov 29, 2016

Me too :)
#458

@tejasbubane
Copy link
Member Author

Yikes! 🙈

@kytrinyx
Copy link
Member

❤️ ❤️ ❤️ to both of you! Let me look at both of these—and if you have an opinion on which we should merge, please speak up.

@kytrinyx
Copy link
Member

I like that this prints the error to STDERR.

I'm on the fence about the progress thing. I actually have a philosophical thingy around CLIs and output (which I don't adhere to strictly, but which I think about a lot). I wrote about it here: http://whipperstacker.com/2015/10/05/silence-noisy-output-from-command-line-tools-by-monitoring-progress-with-siginfo/

I think in the case of x-common and builds and things it makes sense to include it, because we're going to be running it in a context where we're impatient for it to complete.

OK, @tejasbubane @Insti -- what do you think? Should we merge one or the other or combine some aspects of both?

@tejasbubane
Copy link
Member Author

I liked @Insti's usage of require_file function. Maybe combine some aspects of both? I think the progress bar would go well with CI. And we should add some success message as well.

@Insti
Copy link
Contributor

Insti commented Nov 30, 2016

Shall we merge mine? It currently mostly works, and then anyone can contribute further improvements. It's a shame we duplicated our efforts on this one.

@tejasbubane
Copy link
Member Author

It's a shame we duplicated our efforts on this one.

Yeah we should call dibs as soon as possible (note to self)

Shall we merge mine? It currently mostly works, and then anyone can contribute further improvements

Sounds good 👍

@Insti
Copy link
Contributor

Insti commented Nov 30, 2016

I've added errors to STDOUT and a --progress command line option. #458

@Insti
Copy link
Contributor

Insti commented Dec 29, 2016

Closed in favor of: #458 which does essentially the same thing.

Thanks for your work on this @tejasbubane, its unfortunate we both worked on the same feature at the same time.

I hope I'm not out of line by merging that one and closing this, but It's been a month, and no-one else was stepping forward to make the decision. If it's wrong we can always revert and do it the other way.

@Insti Insti closed this Dec 29, 2016
@tejasbubane tejasbubane deleted the filecheck branch December 29, 2016 16:23
@tejasbubane
Copy link
Member Author

No worries @Insti 👍

emcoding pushed a commit that referenced this pull request Nov 19, 2018
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.

3 participants