Skip to content

Conversation

@Anmol202005
Copy link
Collaborator

@Anmol202005 Anmol202005 commented Jul 28, 2025

Fixes: #68
Changed recipe logic to prepend header to the existing comments.

@Anmol202005 Anmol202005 force-pushed the headerUpdate branch 2 times, most recently from ba3b0a4 to 2b58c87 Compare July 28, 2025 14:57
Copy link
Collaborator

@timurt timurt left a comment

Choose a reason for hiding this comment

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

One thought to consider

// This file is part of the Checkstyle OpenRewrite test suite.
///////////////////////////////////////////////////////////////////////////////////////////////

///////////////////////////////////////////////////////////////////////////////////////////////
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am okay with the approach, but just always prepending sounds like we might miss some opportunities

if the input file has no header -> prepend
If the input file has a header and the number of lines mismatches config header -> prepend
if the input file has a header and the number of lines is the same -> override

Something that can be considered later

Copy link
Member

Choose a reason for hiding this comment

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

@Anmol202005 please create issue so we track it and refer to it. We don't have much data yet to decide what's the best way to handle this. Let's keep it as issue for later

Copy link
Member

Choose a reason for hiding this comment

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

@Anmol202005 please share a link to issue

@rdiachenko rdiachenko assigned rdiachenko and unassigned timurt Jul 30, 2025
Copy link
Member

@rdiachenko rdiachenko left a comment

Choose a reason for hiding this comment

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

lgtm

@rdiachenko rdiachenko merged commit 9389bf1 into checkstyle:main Jul 31, 2025
3 checks passed
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.

Update header implementation

4 participants