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

Update composer deps and update php-cs-fixer conf #1881

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

anvit
Copy link
Contributor

@anvit anvit commented Oct 10, 2024

Update composer dependencies:

  • jasig/phpcas is no longer maintained. apereo/phpcas is the suggested replacement.
  • Updating to php-cs-fixer 3.64.0 introduces some new rules which would lead to changes in over ~400 files. Selectively changing or turning off some of these rules to avoid noise and to maintain the current standard in AtoM

@anvit anvit self-assigned this Oct 10, 2024
@anvit anvit added the dependencies Pull requests that update a dependency file label Oct 10, 2024
@anvit anvit added this to the 2.9.0 milestone Oct 10, 2024
@anvit anvit requested a review from a team October 10, 2024 22:09
@jraddaoui
Copy link
Contributor

Nice @anvit!

At the time, we decided to follow the rules giving by php-cs-fixer and the community as much as possible and avoid our own opinions/discussions. I'd try to follow those new rules when possible too. I'd suggest you continue this PR and enable them one by one to see what they do and, unless you have a really strong argument against them, follow the default as much as possible. Check the comment in the config to know why we changed only one rule. Maybe with the new version that rule could be restored to the default too.

Copy link
Contributor

@jraddaoui jraddaoui left a comment

Choose a reason for hiding this comment

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

Thanks @anvit!

I think they are mostly nice changes, always tricky in templates, but it's nice.

@@ -116,7 +116,7 @@ public function validateFilename($filename): string
return $filename;
}

public function setOptions(array $options = null): void
public function setOptions(?array $options = null): void
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL about nullable type syntactic sugar.

@@ -199,7 +199,7 @@
<section class="actions">
<ul>

<?php if (QubitAcl::check($resource, 'update') || (QubitAcl::check($resource, 'translate'))) { ?>
<?php if (QubitAcl::check($resource, 'update') || QubitAcl::check($resource, 'translate')) { ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing how many unneeded parenthesis we had.

Update composer dependencies:
- jasig/phpcas is no longer maintained. apereo/phpcas is the suggested
  replacement.
- Updating to php-cs-fixer 3.64.0 introduces some new rules which would
  lead to changes in over ~400 files. Selectively changing or turning
  off some of these rules that would lead to problems.
@anvit anvit merged commit e42994a into qa/2.x Oct 21, 2024
6 checks passed
@anvit anvit deleted the dev/update-composer-deps branch October 21, 2024 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants