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

[stable27] fix(Users/Quota setting): Prevent floating point value from getting truncated in locales other than english #42172

Merged
merged 2 commits into from Jan 18, 2024

Conversation

marcelklehr
Copy link
Member

Checklist

@marcelklehr
Copy link
Member Author

/compile

@marcelklehr marcelklehr force-pushed the fix/quota-with-non-english-locale-stable27 branch from b16eb6c to fced4c9 Compare December 12, 2023 11:48
@marcelklehr
Copy link
Member Author

/compile

@marcelklehr
Copy link
Member Author

/backport fced4c9 to stable26

@@ -30,11 +30,11 @@
<NcLoadingIcon v-if="loading"
class="menu-entry__loading-icon"
:size="18" />
<img v-else :src="cachedIcon" alt="" />
<img v-else :src="cachedIcon" alt="">
Copy link
Member

Choose a reason for hiding this comment

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

In future let's not add unrelated changes in backports @marcelklehr ;)

@blizzz blizzz mentioned this pull request Jan 15, 2024
@blizzz
Copy link
Member

blizzz commented Jan 15, 2024

new conflicts

@marcelklehr marcelklehr force-pushed the fix/quota-with-non-english-locale-stable27 branch 2 times, most recently from fced4c9 to f0d8dbd Compare January 15, 2024 10:43
@marcelklehr
Copy link
Member Author

/compile

@marcelklehr
Copy link
Member Author

/backport f0d8dbd to stable26

@marcelklehr
Copy link
Member Author

/compile

1 similar comment
@marcelklehr
Copy link
Member Author

/compile

@blizzz
Copy link
Member

blizzz commented Jan 15, 2024

Binary files a/dist/6799-6799.js and /dev/null differ

I have questions 😅

@blizzz blizzz force-pushed the fix/quota-with-non-english-locale-stable27 branch from f0d8dbd to 6cfcded Compare January 15, 2024 20:32
@blizzz
Copy link
Member

blizzz commented Jan 17, 2024

CI is not satisfied

…runcated in locales other than english

fixes #18468

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr force-pushed the fix/quota-with-non-english-locale-stable27 branch from 6cfcded to fa2cf8d Compare January 18, 2024 08:11
@marcelklehr
Copy link
Member Author

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@marcelklehr
Copy link
Member Author

Cypress failure seems to be unrelated

@blizzz blizzz merged commit 82fce12 into stable27 Jan 18, 2024
37 of 39 checks passed
@blizzz blizzz deleted the fix/quota-with-non-english-locale-stable27 branch January 18, 2024 12:24
@backportbot-nextcloud
Copy link

The backport to stable26 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable26
git pull origin stable26

# Create the new backport branch
git checkout -b fix/foo-stable26

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts. Resolve them.
git cherry-pick abc123

# Push the cherry pick commit to the remote repository and open a pull request
git push origin fix/foo-stable26

Error: Unknown error

More info at https://docs.nextcloud.com/server/latest/developer_manual/getting_started/development_process.html#manual-backport

@marcelklehr
Copy link
Member Author

/skjnldsv-backport fa2cf8d to stable26

@@ -44,7 +44,7 @@
"@nextcloud/capabilities": "^1.0.4",
"@nextcloud/dialogs": "^4.2.2",
"@nextcloud/event-bus": "^3.0.2",
"@nextcloud/files": "^3.0.0-beta.8",
Copy link
Member

Choose a reason for hiding this comment

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

@marcelklehr there are HUGE breaking changes between 3.0 beta and final. This is why we never upgrade for old versions. This change broke 27.1.6.

I would have been asked for review, this PR would have been blocked imho.
The great amount of changes in the dist folder is not a proof of a stable fix for an old stable branch. Please avoid this in the future 🙏

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm sorry for breaking v27.1.6. I only do what I think is reasonable, obviously, so I cannot promise to avoid something like this in the future. As you say, this should have been caught in the review process. Since it hasn't been, maybe the process is flawed.

Copy link
Member

Choose a reason for hiding this comment

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

Not promising to not create bugs is perfectly fine, I did a good one for 28 last week too 😁
I was mostly referring to breaking-change upgrades for old stable branches 😉

@marcelklehr
Copy link
Member Author

Can we revert just the package.json change? I'm afraid I will mess up the package-lock.json

@skjnldsv
Copy link
Member

Can we revert just the package.json change? I'm afraid I will mess up the package-lock.json

We need to discuss this with @susnux, because other areas might be broken 🤔

@susnux
Copy link
Contributor

susnux commented Jan 30, 2024

There are some behavior changes in @nextcloud/files between that beta and the stable change, so it might be that currently some filepicker stuff is broken. If we not revert then maybe just update the nextcloud/dialogs 4.x branch to also be compatible with files v3

@marcelklehr
Copy link
Member Author

I think we should revert to nextcloud/files beta if that's how it was supposed to be in 27. I was mostly concerned about the other changes in this PR, as they are fixing a bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants