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

travis: add check for line endings #2281

Closed
OlegHahm opened this issue Jan 12, 2015 · 11 comments
Closed

travis: add check for line endings #2281

OlegHahm opened this issue Jan 12, 2015 · 11 comments
Assignees
Labels
Area: tests Area: tests and testing framework Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Comments

@OlegHahm
Copy link
Member

Add a script that check for valid UNIX line endings.

@OlegHahm OlegHahm added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Jan 12, 2015
@OlegHahm OlegHahm added this to the Release NEXT MAJOR milestone Jan 12, 2015
@LudwigKnuepfer LudwigKnuepfer added the Area: tests Area: tests and testing framework label Jan 12, 2015
@kushalsingh007
Copy link
Member

I will be working on this issue , before I start just a question , once the script is run what should be the expected output format (should there be a list of the places where UNIX line endings are not used) ?
Example format--
error: Filename: Line number

@kushalsingh007
Copy link
Member

Do tell if the following code looks good , tell the changes that need to be done (if any) so that I can create a PR for it. ( Note : I removed the initial copyright part from now , I think there wont be any changes for that part )

Here is the link to the code I have written

https://gist.github.com/kushalsingh007/2cdbc043b649550c8420

@jnohlgard
Copy link
Member

@kushalsingh007 I think printing the name of the offending file is enough to know what to do. Most likely the entire file is Windows line endings and then it won't be much help to print every line number.

@jnohlgard
Copy link
Member

I am sorry, but why do we need to implement anything at all for this? egrep handles it just fine:

egrep '^M$' . -R -l

where ^M is the character that comes up when you press ctrl+v, enter in the terminal.

Edit: I mean implement other than wrapping egrep for Travis

@kushalsingh007
Copy link
Member

So @gebart should I then create a bash script which uses egrep ? I guess I should waited for someone before starting the task to elaborate the comment :( . Regardless , the script also works fine doesn't it ?
I'll make those changes right away printing the name of the file only. And so , no PR for this one then (or is there still hope left :P ) ?

@jnohlgard jnohlgard assigned OlegHahm and unassigned Kijewski Mar 5, 2015
@jnohlgard
Copy link
Member

@kushalsingh007 that's just my opinion. Since this script will have to be run for every PR and every commit made to the RIOT repo on the overloaded Travis CI servers, it is better to use the option which has the least overhead, which I guess is egrep or some other tool which has been around for years instead of writing our own.

Writing this kind of tool is a good student assignment but I think for everyday usage in our CI it is better to go with something tried and tested.

Assigned @OlegHahm since @Kijewski has not been very active on RIOT lately.

@kushalsingh007
Copy link
Member

@gebart Here is the updated python script regardless https://gist.github.com/kushalsingh007/2cdbc043b649550c8420
I think what you say sounds correct :)
, should I write a bash script using egrep now , or wait till @OlegHahm comes ?

@OlegHahm
Copy link
Member Author

OlegHahm commented Mar 5, 2015

Yes, I agree with @gebart that egrep aka grep -E seems to be the best solution

@kushalsingh007
Copy link
Member

Have a look at the bash script and give your feedback
https://gist.github.com/kushalsingh007/05e50ade69ea3c98ee6f

Edit : I have created a PR for it :)

kushalsingh007 added a commit to kushalsingh007/RIOT that referenced this issue Mar 5, 2015
-Created a bash script using grep to check for carriage return at line endings.
-Fix for issue RIOT-OS#2281
@kushalsingh007
Copy link
Member

I think this issue can be closed now.

@miri64
Copy link
Member

miri64 commented Mar 10, 2015

Yes. One Github-Tip: If you reference an issue to be fixed in an commit or pull request like "Fixes #" or "Closes #" the issue gets to be closed automatically as soon as it gets merged ;-)

@miri64 miri64 closed this as completed Mar 10, 2015
@miri64 miri64 added the Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented label Sep 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework Community: good first issue This issue is good for newcomers to RIOT to be addressed / implemented Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

No branches or pull requests

7 participants