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

Use latest php-cs-fixer 2.17.1 #528

Merged
merged 3 commits into from
Dec 14, 2020

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Dec 13, 2020

The latest php-cs-fixer finds some new things. Code changes were needed in sabre-io/dav#1316 for sabre/dav. We might as well consistently make sure to use at least this version 2.17.1 in all repos so that we are doing consistent code-style checking.

It complains about:

  • underscore _ in method names.
  • use of die() (wants it to be exit())
  • blank lines in switch-case
  1. adjust code to make php-cs-fixer happy
  2. Update windowszones timezone data to 2020-12-13
  3. Fix various typos

@phil-davis phil-davis self-assigned this Dec 13, 2020
@phil-davis phil-davis force-pushed the use-latest-php-cs-fixer branch from c90e244 to 2fd911f Compare December 13, 2020 07:50
@codecov
Copy link

codecov bot commented Dec 13, 2020

Codecov Report

Merging #528 (96d330d) into master (2e4e5d6) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #528   +/-   ##
=========================================
  Coverage     98.69%   98.69%           
  Complexity     1757     1757           
=========================================
  Files            66       66           
  Lines          4277     4277           
=========================================
  Hits           4221     4221           
  Misses           56       56           
Impacted Files Coverage Δ Complexity Δ
lib/Component/VCalendar.php 94.59% <ø> (ø) 64.00 <0.00> (ø)
lib/Component/VCard.php 100.00% <ø> (ø) 53.00 <0.00> (ø)
lib/FreeBusyGenerator.php 98.10% <ø> (ø) 90.00 <0.00> (ø)
lib/Parameter.php 99.20% <ø> (ø) 81.00 <0.00> (ø)
lib/Parser/MimeDir.php 99.54% <ø> (ø) 82.00 <0.00> (ø)
lib/Property/Boolean.php 53.84% <ø> (ø) 6.00 <0.00> (ø)
lib/Property/ICalendar/CalAddress.php 100.00% <ø> (ø) 3.00 <0.00> (ø)
lib/Property/ICalendar/DateTime.php 100.00% <ø> (ø) 44.00 <0.00> (ø)
lib/Property/IntegerValue.php 100.00% <ø> (ø) 5.00 <0.00> (ø)
lib/Property/VCard/LanguageTag.php 100.00% <ø> (ø) 3.00 <0.00> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e4e5d6...96d330d. Read the comment docs.

@phil-davis
Copy link
Contributor Author

@DeepDiver1975 @staabm please review. I do not feel comfortable merging it directly - there are quite a few lines of diff.

Comment on lines -47 to +46
exec(__DIR__.'/sabre-cs-fixer fix '.escapeshellarg($outputFile));
exec(__DIR__.'/../vendor/bin/php-cs-fixer fix '.escapeshellarg($outputFile));
Copy link
Member

Choose a reason for hiding this comment

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

hmm not sure its correct that this path changed? could you verify this bin still works?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ran fetch_windows_zones.php locally from the root of the repo in order to update the windows zones file.
That is how I found that the old sabre-cs-fixer is no longer, and made it work by doing this.

Copy link
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

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

except the one mentioned line, it looks good to me.

thanks!

@phil-davis phil-davis merged commit da42738 into sabre-io:master Dec 14, 2020
@phil-davis phil-davis deleted the use-latest-php-cs-fixer branch December 14, 2020 09:00
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