Skip to content

Comments

update contributing#3284

Merged
SaitTalhaNisanci merged 2 commits intomasterfrom
update/contributing
Dec 11, 2019
Merged

update contributing#3284
SaitTalhaNisanci merged 2 commits intomasterfrom
update/contributing

Conversation

@SaitTalhaNisanci
Copy link
Contributor

For style and some other style related checks we have 3 checks. citus_indent is just one of them. We should run make reindent which runs all of the checks. This PR updates the contributing.md with this information.

@SaitTalhaNisanci SaitTalhaNisanci added the documentation to track issues that needs documentation change label Dec 11, 2019
@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #3284 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3284      +/-   ##
==========================================
- Coverage   92.05%   92.03%   -0.03%     
==========================================
  Files         163      163              
  Lines       32865    32865              
==========================================
- Hits        30254    30247       -7     
- Misses       2611     2618       +7

@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #3284 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3284      +/-   ##
==========================================
+ Coverage   92.04%   92.06%   +0.01%     
==========================================
  Files         163      163              
  Lines       32865    32865              
==========================================
+ Hits        30251    30256       +5     
+ Misses       2614     2609       -5

@onderkalaci
Copy link
Contributor

I get the following error on Mac:

make reindent
/Users/onderkalaci/Documents/citus_code/citus/ci/fix_style.sh
sed: -i may not be used with stdin
make: *** [reindent] Error 1

@SaitTalhaNisanci
Copy link
Contributor Author

I think it is because of this line https://github.com/citusdata/citus/blob/master/ci/editorconfig.sh#L10

The script I wrote uses editorconfig as well, I thought it would already work on Mac as well but it seems that it doesnt.

So if you get a problem with editorconfig, how do you fix it then as you cannot run this script on Mac, do you fix it manually? Or you never get an error because you already have your editor configured to fix style?

@onderkalaci
Copy link
Contributor

So if you get a problem with editorconfig, how do you fix it then as you cannot run this script on Mac, do you fix it manually? Or you never get an error because you already have your editor configured to fix style?

it happened few times, I fixed them manually by removing -i and doing some more tricks. I agree that this is not the problem of make reindent but it wouldn't work until we fix the issue.
I can look that later

@SaitTalhaNisanci
Copy link
Contributor Author

I will merge this PR but it would be useful you can do a patch for that. I dont have a macbook so I dont know how to fix it.

@SaitTalhaNisanci SaitTalhaNisanci merged commit 3422e79 into master Dec 11, 2019
@SaitTalhaNisanci SaitTalhaNisanci deleted the update/contributing branch December 11, 2019 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation to track issues that needs documentation change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants