Rewrites/type-hints App and array #600
No reviewers
Labels
No labels
2018.09
2019.01
2019.03
2019.06
2019.09
2019.12
2020.03
2020.06
2020.09
2020.12
2021.03
2021.07
2021.09
2022.02
2022.06
2022.09
2022.12
2023.04
2023.05
2023.09
2024.03
2024.06
dependencies
Hackathon 2021
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: friendica/friendica-addons#600
Loading…
Reference in a new issue
No description provided.
Delete branch "rewrites/type-hints-app"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Added type-hints for safer code also
&$ais being replaced.Uncaught TypeError: Argument 2 passed to buglink_active() must be of the type array, string given- ops, need to fix those. :-(Since we are now in the RC phase, I suggest that we merge this after the RC changes are merged back into the RC.
Please check my comments.
Concerning your PR: I really, really would prefer having a PR per addon - and then we could really work through the addons to make them perfect. So we are having many addons in a single PR and they only cover some stuff - but not all. And while reviewing them I already know that I won't question any formatting or style issue (but only in function issues), since this would lead in endlessly repeating review processes. And of course you have to expect to solve many merge conflicts, since work at the addons will continue from other sides.
And since reviewing 60+ files will take some time, expect me to do this maybe once a week. Currently I have to focus on implementing stuff. There is so much work to do especially with the item storage.
Okay, next time more PRs than just a complete overhaul.
Pushed an update.
Please stop adding anything to this PR, it's large enough already. Can you revert your last commit and just care about the reviews? ijpost is the perfect example of file that should be completely cared for separately, including fixing the indentation.
Oh, sorry. Okay.
Have undone some parts and single-committed almost each file.
If you're writing 20 commits per PR, can you please distinguish themselves from each other? You can write the specific fix you're doing, and/or the specific file/method you're changing.
Please, please, please, either stick to a single change over multiple files, or multiple changes in a few files. This PR does both and it was a hassle to review, especially since not all the changes were consistent, which means I had to be really careful over an extended period of time.
Okay, should we then better close this and I do the "magic" of splitting it up?
Well, I'm mostly commit quickly. Maybe I can add
[addon-name]in first line of each commit and create PRs per addon as requested?Yes, please. The review should be much quicker for each addon than it is for the whole here.
This PR has conflicts with de
developbranch.I should then fix this, maybe this weekend.
Ga, oversaw it. I notify myself @Quix0r ...
Okay, done. And yes, I rebased. ;-)
~Parser errors!~ Fixed.
Once more you did a little too much more than what the PR advertises. It's okay, I'm doing the same, but please stick to a single change by PR next time. Either type-hints or missing braces or missing spaces for example.
Additionally, you broke half a dozen addons by changing hook function prototypes.
Okay, so far finished all. Do you may have an example for a broken one?
Every time you removed the
&from a hook function parameter. I mentioned them all in my review above.Ah, okay. Let's finish this. And then I start making new PRs for each addon individually:>
x()is deprecated? Then ~remove~ replace withisset()/empty()(rules here?)Apporarray)That's a good list already!
Thanks for agreeing with my demanding terms! Are you going to submit a patch against friendica to fix the include/conversation bug? We can't merge this until it's fixed.
Which hook do I have to look in? Edit:
display_item. ;-)callSingleHookalso acceptsnullfor$dataand no type-hint there. I think I open another issue over there. ;-)Blocked by https://github.com/friendica/friendica/issues/5335
Okay, conflict fixed.
New day, new conflicts.
Yay!
rebasedid its job again, messing with microsuck-hub around ... :-( I think I better close this one and re-do it on a per-addon basis as requested by @MrPetovanPull request closed