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

Cast to int #32838

Merged
merged 1 commit into from
Nov 8, 2022
Merged

Cast to int #32838

merged 1 commit into from
Nov 8, 2022

Conversation

markuszeller
Copy link
Contributor

@markuszeller markuszeller commented Jun 12, 2022 •
edited by kesselb
Loading

Fix #32837

@markuszeller markuszeller mentioned this pull request Jun 12, 2022
Closed
8 tasks
@markuszeller
Copy link
Contributor Author

Issue #32837

@kesselb
Copy link
Contributor

kesselb commented Jun 12, 2022

Thanks @markuszeller ,

looks like #32059 / #32841?

@markuszeller
Copy link
Contributor Author

Thanks @markuszeller ,

looks like #32059 / #32841?

Line 292 needs to be casted as well. That's not the case in both mentioned merges.

@kesselb
Copy link
Contributor

kesselb commented Jun 13, 2022 •
edited
Loading

Thanks @markuszeller ,
looks like #32059 / #32841?

Line 292 needs to be casted as well.

image

Please add the cast in L292 or L294 (for the last calculation) but not both. Thanks 👍

@markuszeller
Copy link
Contributor Author

See https://github.com/nextcloud/server/security/code-scanning/14138

Getting a 404.

@kesselb
Copy link
Contributor

kesselb commented Jun 13, 2022

See RedundantCast

Getting a 404.

Added a screenshot.

@kesselb
Copy link
Contributor

kesselb commented Jun 13, 2022

Thanks 👍

Would you mind to sign off your commits? https://github.com/nextcloud/server/pull/32838/checks?check_run_id=6857948898

markuszeller
markuszeller commented Jun 13, 2022
View reviewed changes
Copy link
Contributor Author

@markuszeller markuszeller left a comment

Choose a reason for hiding this comment

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

Need to cast $secs.

@markuszeller markuszeller force-pushed the bugfix/32837/implicitFloatConversion branch from 7c54805 to 73c000d Compare June 13, 2022 08:54
@kesselb kesselb requested a review from st3iny June 13, 2022 09:07
@markuszeller markuszeller force-pushed the bugfix/32837/implicitFloatConversion branch from 73c000d to d648ee7 Compare June 13, 2022 13:27
@kesselb
Copy link
Contributor

kesselb commented Nov 7, 2022

Hi @markuszeller,

Could you please rebase your branch to pull in the latest changes?

Thanks and sorry it took so long from our side.

@tcitworld
Copy link
Member

The rebase seems to have gone wrong :(

@markuszeller
Copy link
Contributor Author

The rebase seems to have gone wrong :(

It asked me to merge master into my branch. Should I redo and force a rebase?

@miaulalala
Copy link
Contributor

Hi @markuszeller can you please checkout your master branch, then do a git pull, then switch back to this branch and do the rebase again? Then you should have a clean commit history.

@markuszeller markuszeller force-pushed the bugfix/32837/implicitFloatConversion branch from 3139ee1 to 41e15f7 Compare November 7, 2022 15:15
@kesselb
Copy link
Contributor

kesselb commented Nov 7, 2022

Hi, please run to fix the dco check:

git rebase HEAD~1 --signoff
git push --force-with-lease origin bugfix/32837/implicitFloatConversion

@markuszeller
Cast to int nextcloud#32837
15289f9 
Signed-off-by: Markus Zeller <git@markuszeller.com>
@markuszeller markuszeller force-pushed the bugfix/32837/implicitFloatConversion branch from 41e15f7 to 15289f9 Compare November 7, 2022 15:45
@kesselb kesselb requested a review from szaimen November 7, 2022 15:49
@markuszeller
Copy link
Contributor Author

Hi, please run to fix the dco check:

git rebase HEAD~1 --signoff git push --force-with-lease origin bugfix/32837/implicitFloatConversion

Thank you for your assistance. Hopefully that's working now.

Can you help me understanding why to do HEAD~1 --signoff, because I think the commit was already signed? At least my PHPStorm is configured to do so.

szaimen
szaimen approved these changes Nov 7, 2022
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

Fine by me

@szaimen szaimen added this to the Nextcloud 26 milestone Nov 7, 2022
@szaimen szaimen added the 3. to review Waiting for reviews label Nov 7, 2022
@szaimen
Copy link
Contributor

szaimen commented Nov 7, 2022

/backport to stable24

@szaimen
Copy link
Contributor

szaimen commented Nov 7, 2022

/backport to stable25

@szaimen
Copy link
Contributor

szaimen commented Nov 7, 2022

/backport to stable23

@kesselb
Copy link
Contributor

kesselb commented Nov 7, 2022

Can you help me understanding why to do HEAD~1 --signoff, because I think the commit was already signed? At least my PHPStorm is configured to do so.

I guess the sign-off was lost due to the rebase.

@szaimen szaimen disabled auto-merge November 8, 2022 09:53
@szaimen szaimen merged commit 175ac79 into nextcloud:master Nov 8, 2022
@welcome
Copy link

welcome bot commented Nov 8, 2022

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 8, 2022
Merged
@kesselb
Copy link
Contributor

kesselb commented Nov 8, 2022

/backport to stable25

@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 8, 2022
Merged
@markuszeller markuszeller deleted the bugfix/32837/implicitFloatConversion branch November 8, 2022 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szaimen szaimen szaimen approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@kesselb kesselb kesselb approved these changes

@st3iny st3iny Awaiting requested review from st3iny

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@miaulalala miaulalala Awaiting requested review from miaulalala

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@tcitworld tcitworld Awaiting requested review from tcitworld

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.

[Bug]: occ files:scan error Implicit conversion from float
7 participants
@markuszeller @kesselb @tcitworld @miaulalala @szaimen @CarlSchwan @skjnldsv