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

Fixes #174. Insert boilerplate correctly where comment on first line. #175

Merged

Conversation

micaherne
Copy link
Contributor

This fixes #174

I've added a test fixture to make sure it works as expected. I'd rather there was a blank line between the boilerplate and the original comment but I can't find a way to do this that doesn't either break other tests, or require a lot of complicated code, so hopefully what it's doing is acceptable.

@micaherne micaherne force-pushed the 174-boilerplate-with-firstline-comment branch from 11337ce to 904ac82 Compare July 2, 2024 16:22
Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.11%. Comparing base (a0bfb12) to head (8965b35).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #175   +/-   ##
=========================================
  Coverage     98.11%   98.11%           
- Complexity      950      951    +1     
=========================================
  Files            40       40           
  Lines          2811     2816    +5     
=========================================
+ Hits           2758     2763    +5     
  Misses           53       53           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@micaherne
Copy link
Contributor Author

I'm not sure what to do about the code coverage failure here. I haven't really changed the lines it's complaining about, just moved them. They'll presumably only be called now if there is a PHPCS tag on the first line but I'm not sure I quite understand how that is supposed to work so I don't think I can provide a test file.

@micaherne micaherne force-pushed the 174-boilerplate-with-firstline-comment branch from 904ac82 to 0ff19bb Compare July 4, 2024 10:27
Copy link
Member

@stronk7 stronk7 left a comment

Choose a reason for hiding this comment

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

Thanks @micaherne , I think that's all we needed, now we have a "proof" of the use case behind that code branch.

I'm approving this now, would be great if the remaining indentation detail is fixed. Will merge then, thanks!

moodle/Tests/FilesBoilerPlateCommentTest.php Outdated Show resolved Hide resolved
@micaherne micaherne force-pushed the 174-boilerplate-with-firstline-comment branch from 0ff19bb to 8965b35 Compare July 4, 2024 12:37
@micaherne
Copy link
Contributor Author

Oops I don't know how I managed that! That's it fixed now :)

@stronk7 stronk7 merged commit f7668d9 into moodlehq:main Jul 4, 2024
13 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.

BoilerplateCommentSniff causes invalid code if first line has comment
2 participants