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.

Sign up for GitHub

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

Jump to bottom

[stable16] Add strict type on Notifications tests #16014

Closed
wants to merge 1 commit into from
Closed

[stable16] Add strict type on Notifications tests #16014

wants to merge 1 commit into from

Conversation

backportbot-nextcloud[bot]
Copy link

backport of #15935

@skjnldsv
Add strict type on Notifications tests
09fa2c6 
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
@backportbot-nextcloud backportbot-nextcloud bot added this to the Nextcloud 16.0.2 milestone Jun 19, 2019
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Jun 19, 2019
Merged
@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Jun 19, 2019
@skjnldsv
Copy link
Member

Fixes broken stable16 NODB tests

@skjnldsv skjnldsv requested a review from blizzz June 19, 2019 13:38
@skjnldsv skjnldsv mentioned this pull request Jun 19, 2019
Merged
@nickvergessen
Copy link
Member

why are we backporting type hints on public interfaces? That will break all tests of apps...

@skjnldsv
Copy link
Member

@nickvergessen because that seemed like a bug ore than a feature? 🙈

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 21, 2019
@nickvergessen
Copy link
Member

Yeah well, doesn't matter. We don't backport breaking the APIs.

We will fix the type hints for 17, for 16 only the cast should be backported and all other changes reverted.

@skjnldsv
Copy link
Member

Sorry about that!
So we merge this?

@georgehrke
Copy link
Member

Keep all the casting (or add additional ones), but remove all the type hints

@skjnldsv
Copy link
Member

Well, this one just fixes the tests.
All the edits are already in

@georgehrke
Copy link
Member

I guess we should revert the back port of the other PR before releasing the next version then.

cc @nickvergessen

@nickvergessen
Copy link
Member

I guess we should revert the back port of the other PR before releasing the next version then.

Yes, exactly. Revert the backport and just keep the casting or make a new commit with the casting:
https://github.com/nextcloud/server/pull/15912/files#diff-ea411f2ca6aeb5624cef170048c91d67R199

Only this one line is necessary to be backported.

@rullzer rullzer mentioned this pull request Jun 24, 2019
Merged
5 tasks
@skjnldsv skjnldsv requested a review from rullzer June 25, 2019 10:28
@rullzer
Copy link
Member

rullzer commented Jun 25, 2019

#16068 is the fix... let have this die

@rullzer rullzer closed this Jun 25, 2019
@rullzer rullzer deleted the backport/15935/stable16 branch June 25, 2019 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@georgehrke georgehrke Awaiting requested review from georgehrke

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@blizzz blizzz Awaiting requested review from blizzz

@rullzer rullzer Awaiting requested review from rullzer

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Milestone
Nextcloud 16.0.2
Development

Successfully merging this pull request may close these issues.
5 participants
@skjnldsv @nickvergessen @georgehrke @rullzer @ChristophWurst