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

Chore/update php cs fixer #600

Merged
merged 2 commits into from
Nov 25, 2021

Conversation

aragon999
Copy link
Contributor

This pull request currently includes #595 (since this does some prior work) and #598 (which I needed since I am running php8) if these two pull requests are merged I am happy to rebase this PR.

@Sebbo94BY
Copy link
Collaborator

Both referenced pull requests have been merged. This requires a rebase now.

@aragon999 aragon999 force-pushed the chore/update-php-cs-fixer branch 3 times, most recently from 00c1aef to cecd897 Compare November 23, 2021 17:30
@aragon999
Copy link
Contributor Author

aragon999 commented Nov 23, 2021

I rebased, but I am quite confused why the merge commit is still shown in this PR.

Locally everything seems fine, as far as I can tell:
grafik

Edit: I just noticed, that the merge is missing for the develop branch. Should I use the master or develop branch as base branch?

@Sebbo94BY
Copy link
Collaborator

The develop branch for now please. :)

The mentioned commits / pull requests were only merged to develop.

I'm already thinking about removing this develop branch as it doesn't really make sense in this project. We could also simply branch from the default branch.

But for now: Develop. :)

@aragon999
Copy link
Contributor Author

Thanks for the feedback.

Yeah I mean the branches really depend on the desired workflow. Personally I actually like to work with a master/trunk branch and only have feature branches (and version branches, if one wants to offer bugfix releases).

@Sebbo94BY
Copy link
Collaborator

Approved - checks are running. One already failed. 🙈

Yeah, there are a lot of different approaches to how to work with branches and every company or whatever is handling it more or less different. :D

@aragon999
Copy link
Contributor Author

Yes, sorry I forgot some changes, and did not check properly what has changed in the develop branch.

I added some changes, but unfortunately locally the psalm command still fails:

  Error on line 12:
    Element '{https://getpsalm.org/schema/config}psalm', attribute 'requireVoidReturnType': The attribute 'requireVoidReturnType' is not allowed.

Although I updated psalm to the version which should include this option. Not sure, hope the pipelines do not fail anymore.

@Sebbo94BY Sebbo94BY requested review from Sebbo94BY and removed request for Sebbo94BY November 23, 2021 21:47
@aragon999
Copy link
Contributor Author

I just checked, for me psalm also fails locally without my changes.

I can also not find the attribute in the XML schema: https://psalm.dev/schema/config

@Sebbo94BY Sebbo94BY merged commit 4e668b4 into barbushin:develop Nov 25, 2021
@Sebbo94BY
Copy link
Collaborator

Let's fix these issues in a different merge request.

@aragon999 aragon999 deleted the chore/update-php-cs-fixer branch November 26, 2021 09:50
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.

None yet

2 participants