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

[PHPCS] Make type hinting mandatory #5396

Closed
wants to merge 8 commits into from
Closed

[PHPCS] Make type hinting mandatory #5396

wants to merge 8 commits into from

Conversation

johnsaigle
Copy link
Contributor

@johnsaigle johnsaigle commented Oct 29, 2019

Brief summary of changes

PHPCS now makes type hinting mandatory for function parameters and return types. You can add them automatically based on PHPDocs using phpcbf. mixed annotations don't result in any changes.

Links to related tickets (GitHub, Redmine, ...)

@johnsaigle johnsaigle added Cleanup PR or issue introducing/requiring at least one clean-up operation Testing PR contains test plan or automated test code (or config files for Travis) Meta PR does something that organizes, upgrades, or manages the functionality of the codebase State: Needs work PR awaiting additional work by the author to proceed labels Oct 29, 2019
@johnsaigle
Copy link
Contributor Author

Adding needs work because this is unlikely to pass Travis for another few rounds.

@johnsaigle
Copy link
Contributor Author

This will be rebased to the "Dev" branch once our new branching system takes effect.

@johnsaigle johnsaigle removed the State: Needs work PR awaiting additional work by the author to proceed label Oct 30, 2019
@ridz1208 ridz1208 added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 6, 2019
@johnsaigle johnsaigle added State: Needs work PR awaiting additional work by the author to proceed and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Nov 13, 2019
@johnsaigle johnsaigle added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Nov 27, 2019
@johnsaigle johnsaigle changed the base branch from minor to major November 27, 2019 16:08
@johnsaigle johnsaigle added State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Nov 27, 2019
@johnsaigle johnsaigle changed the base branch from major to master December 3, 2019 18:07
@johnsaigle johnsaigle removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Dec 3, 2019
@johnsaigle
Copy link
Contributor Author

This is now incorporating changes from 22 but there are many phan errors related to this to fix.

@johnsaigle johnsaigle added State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Dec 9, 2019
@ridz1208
Copy link
Collaborator

@johnsaigle wouldn't it be easier to tackle this in smaller chunks ?

@johnsaigle
Copy link
Contributor Author

johnsaigle commented Dec 18, 2019

@ridz1208 Maybe but not after I've put all this time into it 🤣

And also I have been doing separate PRs for this. There's a whole project about it https://github.com/aces/Loris/projects/11. It would be good to automate this once and for all.

I think PRs adding PHPCS and Phan rules are always pretty big but we can sort it out. We're not releasing anytime soon so even if there are some type errors we can get them fixed.

@johnsaigle johnsaigle added the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Dec 18, 2019
@ridz1208
Copy link
Collaborator

@johnsaigle I'm more concerned about travis

@johnsaigle
Copy link
Contributor Author

Let me handle Travis 😈

@johnsaigle johnsaigle removed the State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) label Dec 18, 2019
@driusan driusan added the Blocking PR should be prioritized because it is blocking the progress of another task label Jan 7, 2020
@johnsaigle johnsaigle added State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Jan 20, 2020
@johnsaigle johnsaigle added State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Jan 29, 2020
@johnsaigle johnsaigle closed this Feb 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocking PR should be prioritized because it is blocking the progress of another task Cleanup PR or issue introducing/requiring at least one clean-up operation Meta PR does something that organizes, upgrades, or manages the functionality of the codebase State: Needs work PR awaiting additional work by the author to proceed Testing PR contains test plan or automated test code (or config files for Travis)
Projects
None yet
3 participants