Skip to content

Issue #13770: Override write func to enforce unix-style newline#13947

Merged
nrmancuso merged 1 commit into
checkstyle:masterfrom
kalpadiptyaroy:enforce-linux-eol
Oct 25, 2023
Merged

Issue #13770: Override write func to enforce unix-style newline#13947
nrmancuso merged 1 commit into
checkstyle:masterfrom
kalpadiptyaroy:enforce-linux-eol

Conversation

@kalpadiptyaroy

@kalpadiptyaroy kalpadiptyaroy commented Oct 24, 2023

Copy link
Copy Markdown
Contributor

For Issue #13770: PR #13774 has a bug, to resolve that we are overriding the write func of PrinterWriter as well to make it concrete.

Proof of Validity - https://ci.appveyor.com/project/Checkstyle/checkstyle/builds/48355264

@kalpadiptyaroy kalpadiptyaroy force-pushed the enforce-linux-eol branch 2 times, most recently from 32acffc to e0b86c0 Compare October 24, 2023 13:50
@romani

romani commented Oct 24, 2023

Copy link
Copy Markdown
Member

Please test this commit in other PR to make sure it works this time

@nrmancuso nrmancuso left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please share a link to successful CI execution in other PR in the PR description here.

Items:

Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java Outdated
Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java Outdated
Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java Outdated
Comment thread src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java Outdated

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ok to merge

@romani

romani commented Oct 24, 2023

Copy link
Copy Markdown
Member

@michael-o , are we doing it in right way?

@michael-o michael-o left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks OK to me. Maybe a test would be good.

@kalpadiptyaroy

Copy link
Copy Markdown
Contributor Author

Looks OK to me. Maybe a test would be good.

We have just verified it by cherry picking it in the other PR.

https://ci.appveyor.com/project/Checkstyle/checkstyle/builds/48355264

@romani

romani commented Oct 24, 2023

Copy link
Copy Markdown
Member

Yes, CI will catch this.
No extra tests are required for now.

@kalpadiptyaroy

Copy link
Copy Markdown
Contributor Author

I will fix the lock tainting error tomorrow morning.

@romani

romani commented Oct 25, 2023

Copy link
Copy Markdown
Member

@kalpadiptyaroy

Copy link
Copy Markdown
Contributor Author

Finally we can close this! Let's merge all CI green!

@nrmancuso nrmancuso merged commit 291506e into checkstyle:master Oct 25, 2023
@nrmancuso

Copy link
Copy Markdown
Contributor

@kalpadiptyaroy please rebase other PR

@kalpadiptyaroy kalpadiptyaroy deleted the enforce-linux-eol branch October 29, 2023 14:02
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