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

General dependency upgrades/bump #72

Merged
merged 7 commits into from
Jul 27, 2022

Conversation

Ocramius
Copy link
Contributor

This is just a general dependency upgrade:

  • Dropped PHP <8.1
  • Allowed symfony/console:^6 (^5 preserved, so that a seamless upgrade is possible for now - to be dropped later)
  • ignored some CS rules that would have caused BC breaks due to missing final marker on some critical classes

Allows following upgrades:

@Ocramius
Copy link
Contributor Author

Ah, I forgot to run phpstan locally -.-

My bad

@Ocramius
Copy link
Contributor Author

@WyriHaximus hopefully something you'd like :D

@WyriHaximus
Copy link
Collaborator

@Ocramius Yes I do love this! Thank you. The only issue I need to fix is that it requires Scrutinizer to be green and that seems to error out 😂

@Ocramius
Copy link
Contributor Author

Kill Scrutinizer!

We already run SA and whatnot, no? :P

@Ocramius
Copy link
Contributor Author

Nvm, changed its config: let's see if it works :)

@Ocramius
Copy link
Contributor Author

@WyriHaximus it is done.

@WyriHaximus
Copy link
Collaborator

Kill Scrutinizer!

We already run SA and whatnot, no? :P

I wouldn't mind, but I'm only maintaining this repo. I can't make those changes, that is up to @jwage.

@WyriHaximus
Copy link
Collaborator

@WyriHaximus it is done.

Aye it is! Except I can't merge it because:
image

@Ocramius
Copy link
Contributor Author

The base branch requires all commits to be signed.

OOOOOOOOOOF.

Not on a computer that can perform signed commits, sorry :-\

@WyriHaximus
Copy link
Collaborator

The base branch requires all commits to be signed.

OOOOOOOOOOF.

Not on a computer that can perform signed commits, sorry :-\

No need to say sorry, just ping me when you are and pushed it with signed commits :).

@Ocramius
Copy link
Contributor Author

Looks like signed commits were enabled relatively recently: no signed commits in master 🤷

Let's wait for @jwage's feedback then.

@jwage
Copy link
Owner

jwage commented Jul 26, 2022

I disabled the signed commits requirement. I am good with killing Scrutinizer too. I am not using it anywhere anymore. No need 👍 Thanks!

@Ocramius
Copy link
Contributor Author

@WyriHaximus your call now 👍

@WyriHaximus WyriHaximus self-requested a review July 27, 2022 22:10
@@ -17,11 +17,9 @@

final class IssueClientTest extends TestCase
{
/** @var RequestFactoryInterface|MockObject */
private $messageFactory;
private RequestFactoryInterface&MockObject $messageFactory;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oeoeoeoeoeoe!

@WyriHaximus
Copy link
Collaborator

@Ocramius Looks good to me. Lets kill Scrutinizer in a follow up PR

@WyriHaximus WyriHaximus merged commit 9f3aab4 into jwage:master Jul 27, 2022
@WyriHaximus WyriHaximus added this to the 1.4.0 milestone Jul 27, 2022
@Ocramius Ocramius deleted the feature/dependency-upgrade branch August 2, 2022 07:49
@Ocramius
Copy link
Contributor Author

Ocramius commented Aug 2, 2022

Thanks @WyriHaximus!

Think this is releasable meanwhile? CI runs on scrutinizer too, so far.

@WyriHaximus
Copy link
Collaborator

Working on releasing this now

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.

3 participants