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 new sniff to detect and remove constructor @return docs #157

Merged
merged 1 commit into from
May 31, 2024

Conversation

andrewnicols
Copy link
Contributor

No description provided.

andrewnicols added a commit to andrewnicols/moodle-cs that referenced this pull request Apr 17, 2024
@andrewnicols andrewnicols force-pushed the constructorShouldNotReturn branch from d0a5198 to a917e01 Compare April 17, 2024 01:30
Copy link

codecov bot commented Apr 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.96%. Comparing base (bc14e30) to head (9573f90).

Current head 9573f90 differs from pull request most recent head 228d21b

Please upload reports for the commit 228d21b to get more accurate results.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #157      +/-   ##
============================================
- Coverage     97.99%   97.96%   -0.04%     
+ Complexity      886      873      -13     
============================================
  Files            39       39              
  Lines          2647     2603      -44     
============================================
- Hits           2594     2550      -44     
  Misses           53       53              

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

andrewnicols added a commit to andrewnicols/moodle-cs that referenced this pull request Apr 17, 2024
@andrewnicols andrewnicols force-pushed the constructorShouldNotReturn branch from a917e01 to a0d7eb1 Compare April 17, 2024 01:32
andrewnicols added a commit to andrewnicols/moodle-cs that referenced this pull request Apr 17, 2024
@andrewnicols andrewnicols force-pushed the constructorShouldNotReturn branch from a0d7eb1 to 74195d7 Compare April 17, 2024 02:32
@andrewnicols
Copy link
Contributor Author

@marinaglancy

Inspired by your docblock fixes.

@stronk7
Copy link
Member

stronk7 commented Apr 17, 2024

LGTM.

The removal looks a little bit complex, more yet, I must confess that I was sure that the fixer had some utility function to remove lines, but I was wrong and it only has functions to add new ones (addNewline() and addNewlineBefore()).

@stronk7
Copy link
Member

stronk7 commented May 20, 2024

pong!

@andrewnicols andrewnicols requested a review from stronk7 May 28, 2024 02:34
andrewnicols added a commit to andrewnicols/moodle-cs that referenced this pull request May 28, 2024
@andrewnicols andrewnicols force-pushed the constructorShouldNotReturn branch from 74195d7 to 7cd3704 Compare May 28, 2024 02:34
@andrewnicols
Copy link
Contributor Author

I agree - there should really be a removeLine!! Drives me mad that there isn't.

@marinaglancy are you able to review this?

andrewnicols added a commit to andrewnicols/moodle-cs that referenced this pull request May 28, 2024
@andrewnicols andrewnicols force-pushed the constructorShouldNotReturn branch from 7cd3704 to 0e513c4 Compare May 28, 2024 02:36
stronk7 pushed a commit to andrewnicols/moodle-cs that referenced this pull request May 31, 2024
@stronk7 stronk7 force-pushed the constructorShouldNotReturn branch from 833478c to b25ab9a Compare May 31, 2024 08:21
@stronk7
Copy link
Member

stronk7 commented May 31, 2024

I've amended the commit to resolve the changelog conflicts.

stronk7 pushed a commit to andrewnicols/moodle-cs that referenced this pull request May 31, 2024
@stronk7 stronk7 force-pushed the constructorShouldNotReturn branch from b25ab9a to 9573f90 Compare May 31, 2024 08:41
@stronk7 stronk7 force-pushed the constructorShouldNotReturn branch from 9573f90 to 228d21b Compare May 31, 2024 16:16
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.

This looks good enough for me, I've rebased it again and going to merge it now, towards a moodle-cs release in minutes... thanks!

@stronk7 stronk7 merged commit a20cb23 into moodlehq:main May 31, 2024
11 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.

2 participants