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

Add .gitattributes. #1798

Merged

Conversation

Oberon00
Copy link
Member

@Oberon00 Oberon00 commented Oct 14, 2020

This fixes spotlessCheck when checking out on windows with git core.eol=crlf and core.autocrlf=true.

Note that for git itself, the eol=lf next to the text=auto changes nothing, but spotless tries to
enforce the core.eol line endings anyway.

Use https://github.com/open-telemetry/opentelemetry-java/pull/1798/files?w=1 (note the ?w=1) to verify that only whitespace (line endings) is changed in the batch files.

This fixes spotlessCheck when checking out on windows with git core.eol=crlf and core.autocrlf=true.

Note that for git itself, the eol=lf next to the text=auto changes nothing, but spotless tries
enforce the core.eol line endings anyway.
@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

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

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1798      +/-   ##
============================================
+ Coverage     85.53%   85.56%   +0.03%     
  Complexity     1374     1374              
============================================
  Files           166      166              
  Lines          5308     5308              
  Branches        549      549              
============================================
+ Hits           4540     4542       +2     
+ Misses          569      568       -1     
+ Partials        199      198       -1     
Impacted Files Coverage Δ Complexity Δ
...try/sdk/metrics/aggregator/LongMinMaxSumCount.java 100.00% <0.00%> (+4.08%) 6.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e539890...8698a02. Read the comment docs.

Copy link
Contributor

@jkwatson jkwatson left a comment

Choose a reason for hiding this comment

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

Thanks! I can't evaluate if this will help windows users or not, but if you say so, then 👍

@Oberon00
Copy link
Member Author

I think you should be able to reproduce the issue on non-Windows by using git config core.eol crlf && git config core.autocrlf true inside your checkout.

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Yeah I think we need a gitattributes file I always have issues on windows without it.

@Oberon00 What do you think about a simpler one like we use in instrumentation repo?

https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/master/.gitattributes

@trask
Copy link
Member

trask commented Oct 15, 2020

+1 to having the same .gitattributes file across both repos (if there is something you think we are missing in instrumentation repo we can update there)

@Oberon00
Copy link
Member Author

The simpler one should also be fine.

@Oberon00
Copy link
Member Author

That said, I think at least the -text should be kept for safety.

Copy link
Contributor

@anuraaga anuraaga left a comment

Choose a reason for hiding this comment

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

Thanks!

@jkwatson
Copy link
Contributor

closing and reopening for the CLA bot

@jkwatson jkwatson closed this Oct 15, 2020
@jkwatson jkwatson reopened this Oct 15, 2020
@jkwatson jkwatson merged commit 62a96ae into open-telemetry:master Oct 15, 2020
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.

4 participants