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

fix(sharing): Remove casting to int from INF #36524

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

nickvergessen
Copy link
Member

Regression from #35736
INF is a the float value INF, casting it to integer will make it 0
https://3v4l.org/IH4Xc

Before After
❌ Red acceptance/features/app-files-sharing-link.feature 🟢 Green acceptance/features/app-files-sharing-link.feature
Bildschirmfoto vom 2023-02-03 15-53-33 Bildschirmfoto vom 2023-02-03 15-53-23

Checklist

@nickvergessen
Copy link
Member Author

Psalm error is fixed by https://github.com/nextcloud/server/pull/36120/files#diff-728c72d2e598172d145e57ed10fdbf1fc107aa73fa75af827418aa16895c3a38R355

@come-nc can I copy that one method to this PR and you rebase afterwards?

@come-nc
Copy link
Contributor

come-nc commented Feb 6, 2023

Psalm error is fixed by #36120 (files)

@come-nc can I copy that one method to this PR and you rebase afterwards?

I’m pretty sure you will get psalm errors somewhere else, no? Do if you must, but I’d rather merge 32bit first.

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Thanks for looking into it 💙

@szaimen
Copy link
Contributor

szaimen commented Feb 6, 2023

Just add the false reported problem to psalms baseline for now?

@nickvergessen
Copy link
Member Author

Just add the false reported problem to psalms baseline for now?

No one is going to remove it.

But will clarify with Come once he arrives at the office

Regression from #35736
INF is a the float INF, casting it to integer will make it 0

Signed-off-by: Joas Schilling <[email protected]>
@nickvergessen nickvergessen force-pushed the bugfix/36510/fix-uploading-to-public-shares branch from 4eb77ff to 80071bd Compare February 7, 2023 13:13
@nickvergessen
Copy link
Member Author

Rebased for green CI

@nickvergessen
Copy link
Member Author

Failing acceptance-header menu is flickering again:

ContactsMenuContext::iSeeThatTheContactsMenuIsShown()
    And I see that the contact "user0" in the Contacts menu is not shown                                                      # ContactsMenuContext::iSeeThatTheContactInTheContactsMenuIsNotShown()
      Failed asserting that true is false.
    And I see that the contact "admin" in the Contacts menu is not shown

@nickvergessen nickvergessen merged commit 82498bc into master Feb 7, 2023
@nickvergessen nickvergessen deleted the bugfix/36510/fix-uploading-to-public-shares branch February 7, 2023 14:59
@blizzz blizzz mentioned this pull request Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can no longer upload files or create folders on public sharing
5 participants