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

Bugfix/213 use relative links #215

Closed

Conversation

digulla
Copy link

@digulla digulla commented Jan 25, 2019

PR to fix issue #213

There are three commits:

Copy link
Contributor

@ppalaga ppalaga left a comment

Choose a reason for hiding this comment

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

Please submit separate PRs for #73 and #213. Please reword the commit messages to Fix #<number> <issueTitle>

@@ -0,0 +1,11 @@
*.cmd text eol=crlf
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please explain what problem is this .gitattributes file trying to solve? In other words, what EOLs do you get now while there are no .gitattributes?

Copy link
Author

Choose a reason for hiding this comment

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

Without this config, Git will checkout all files with the platform line ending (CRLF on Windows).
Also, the mvnw.cmd batch script was committed with Unix line endings (LF). WIth this file, the line endings of .cmd files will be correct on Linux as well.

Over the years, I've tried many approaches to fix line endings in projects and that's the best solution I've found so far.

Copy link
Contributor

Choose a reason for hiding this comment

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

I do not think this is the default git behavior. Do you happen to have core.autocrlf true somewhere in your git config? You can check that via git config --list --show-origin. In case you do have core.autocrlf true for whatever reason, then you could override it to false for the current project by running git config --local core.autocrlf false in the current project directory.

I must say I prefer not adding a file that that will have to be kept in sync with .editorconfig. I prefer .editorconfig to be the single source of truth for line endings.

@digulla
Copy link
Author

digulla commented Jan 31, 2019

I don't want to proceed until we have agreed on a solution.

Since I can't build the project without the git config fix, can I put that into the fix for #73?
Or do you want me to create a PR which I can't build? I mean cherry picking 94d94be should be safe enough but I don't like it.

#215 depends on #73 for me, so same situation. I really don't understand why you can run the IT tests using mvnw and I can't. Our classpaths should be the same and the tests should fail for you, too!

ppalaga pushed a commit to ppalaga/mojohaus-license-maven-plugin that referenced this pull request Feb 1, 2019
ppalaga pushed a commit to ppalaga/mojohaus-license-maven-plugin that referenced this pull request Feb 1, 2019
third-party-report.html

Cherry-picked and reworded from
mojohaus#215 by @digulla
@ppalaga
Copy link
Contributor

ppalaga commented Feb 1, 2019

The two issue fixes from here were moved and slightly adapted in #220 and #221. Closing this one. Thanks for your work @digulla !

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.

Links in aggregate-third-party-report.html point to third-party-report.html
2 participants