Skip to content

Still works on PHP 7.1#226

Merged
greg0ire merged 2 commits intodoctrine:8.1.xfrom
beberlei:Php7.1Support
Oct 25, 2020
Merged

Still works on PHP 7.1#226
greg0ire merged 2 commits intodoctrine:8.1.xfrom
beberlei:Php7.1Support

Conversation

@beberlei
Copy link
Copy Markdown
Member

@beberlei beberlei commented Oct 24, 2020 •
edited
Loading

Restricting the 8.1 branch to 7.1 is an artificial constraint, the code works on 7.1 just fine.

#127 removed 7.1 support, but slevomat/coding-standard reintroduced 7.1 support with slevomat/coding-standard@b26cb5c#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34

malarzm
malarzm previously approved these changes Oct 24, 2020
View reviewed changes
greg0ire
greg0ire reviewed Oct 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@greg0ire greg0ire left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Should 7.1 be added to

coding-standard/.github/workflows/continuous-integration.yml

Lines 92 to 95 in b27f98f

php-version:
- "7.2"
- "7.3"
- "7.4"

?

Should this be changed to 7.1?

coding-standard/.github/workflows/continuous-integration.yml

Line 128 in b27f98f

- "7.2"

@beberlei beberlei mentioned this pull request Oct 24, 2020
Merged
@beberlei
Copy link
Copy Markdown
Member Author

beberlei commented Oct 24, 2020

@greg0ire yes to both, added and works.

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Oct 24, 2020 •
edited
Loading

Note to the person that merges this: you will have to mark "Test fixes (7.2)" as no longer required, and the new 7.1 jobs as required in the settings just before merging this.

@greg0ire greg0ire mentioned this pull request Oct 24, 2020
Merged
@greg0ire greg0ire merged commit 3cf6e3d into doctrine:8.1.x Oct 25, 2020
@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Oct 25, 2020

Thanks @beberlei !

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Oct 25, 2020

Ah crap, this shouldn't have been merged in 8.1.x, it's not a bugfix… Also, master is aliased to 7.0.x 🤦
Let me figure out if master has any BC-breaking changes and I will figure out what to do from there.

@beberlei
Copy link
Copy Markdown
Member Author

beberlei commented Oct 25, 2020

@greg0ire you could argue its a bugfix, as 7.1 support was wrongly removed :-)

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Oct 25, 2020

Yeah sure. I think there are no BC-breaks on master. Also, master has f6d859e , which I believe would have also solved the CI issue we had 😆 . I will merge 8.1x into master, then rename master to 8.2.x, and then we can release 8.2.0

@greg0ire greg0ire added this to the 8.2.0 milestone Oct 25, 2020
@github-actions github-actions bot mentioned this pull request Oct 25, 2020
Merged
@simPod simPod mentioned this pull request Jun 19, 2022
Merged
@greg0ire greg0ire mentioned this pull request Jul 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greg0ire greg0ire greg0ire approved these changes

@SenseException SenseException SenseException approved these changes

@malarzm malarzm malarzm approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

8.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@beberlei @greg0ire @SenseException @malarzm