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

Add "increment-range" option #274

Merged
merged 1 commit into from
Oct 11, 2021
Merged

Add "increment-range" option #274

merged 1 commit into from
Oct 11, 2021

Conversation

IljaN
Copy link
Contributor

@IljaN IljaN commented May 18, 2021 •
edited
Loading

Fixes #279

@mmattel
Copy link
Contributor

mmattel commented May 18, 2021

Please file a doc issue in case close to merge
@EParzefall fyi

@IljaN IljaN force-pushed the restoreEncryptedVersion branch from eae7103 to f52da91 Compare May 19, 2021 10:25
@delete-merged-branch delete-merged-branch bot deleted the branch master May 19, 2021 11:55
@IljaN IljaN marked this pull request as ready for review May 27, 2021 11:58
@IljaN IljaN changed the base branch from restoreEncryptedVersion to master May 28, 2021 10:34
@IljaN IljaN force-pushed the add_option_for_max_increment branch 2 times, most recently from 280dd47 to 9c3ae34 Compare May 28, 2021 10:38
@mmattel mmattel closed this May 28, 2021
@mmattel mmattel reopened this May 28, 2021
@mmattel
Copy link
Contributor

mmattel commented May 28, 2021

Sorry, accidently pressed the wrong key and it got closed...

@IljaN IljaN self-assigned this May 28, 2021
@mmattel
Copy link
Contributor

mmattel commented Jun 2, 2021

@IljaN any update on this (see also my comment / text suggestions above)

@IljaN IljaN force-pushed the add_option_for_max_increment branch from 6d7837b to 7b8898a Compare June 4, 2021 11:03
@IljaN IljaN requested a review from mmattel June 4, 2021 11:03
@IljaN IljaN mentioned this pull request Jun 4, 2021
Closed
@IljaN IljaN force-pushed the add_option_for_max_increment branch from 7b8898a to 37cdb76 Compare June 4, 2021 11:45
@IljaN IljaN added this to the development milestone Jun 4, 2021
@IljaN IljaN force-pushed the add_option_for_max_increment branch from 37cdb76 to b7e625d Compare August 12, 2021 14:07
@ownclouders
Copy link
Contributor

💥 Acceptance tests pipeline apiCoreMKey-50-34-master-mysql8.0-php7.2 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/encryption/2041/65/1

@mmattel
Copy link
Contributor

mmattel commented Aug 12, 2021 •
edited
Loading

@phil-davis could you take a look why the acceptance tests are failing?
(and approve the PR if you are fine with?)

@mmattel
Copy link
Contributor

mmattel commented Sep 2, 2021

Any update on the failing acceptance tests?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@IljaN
Copy link
Contributor Author

IljaN commented Sep 8, 2021 •
edited
Loading

Looks like they could pass now 🙏 . Thanks for checking!

@mmattel
Copy link
Contributor

mmattel commented Sep 8, 2021

@IljaN Before merging, please add a docs issue because we need to add the new occ command component

@phil-davis
Copy link
Contributor

Looks like they could pass now pray . Thanks for checking!

If they fail, it will be because owncloud/core#39180 is needed. But I think the test pipelines against oC10 core latest (10.8.0) are only run in nightly CI - so you might be lucky.

@IljaN IljaN requested a review from JammingBen September 8, 2021 08:58
JammingBen
JammingBen suggested changes Sep 8, 2021
View reviewed changes
Copy link
Contributor

@JammingBen JammingBen left a comment

Choose a reason for hiding this comment

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

Code LGTM, just minor semantics.

lib/Command/FixEncryptedVersion.php
@@ -135,7 +149,7 @@ private function walkPathOfUser($user, $path, OutputInterface $output) {
* @param OutputInterface $output
* @param bool $ignoreCorrectEncVersionCall, setting this variable to false avoids recursion
*/
private function verifyFileContent($path, OutputInterface $output, $ignoreCorrectEncVersionCall = true) {
private function verifyFileContent($path, OutputInterface $output, $incrRange, $ignoreCorrectEncVersionCall = true) {
Copy link
Contributor

Choose a reason for hiding this comment

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

$incrRange missing in docstring

lib/Command/FixEncryptedVersion.php
@@ -170,7 +184,7 @@ private function verifyFileContent($path, OutputInterface $output, $ignoreCorrec
* @param OutputInterface $output
* @return bool
*/
private function correctEncryptedVersion($path, OutputInterface $output) {
private function correctEncryptedVersion($path, OutputInterface $output, $incrRange) {
Copy link
Contributor

Choose a reason for hiding this comment

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

$incrRange missing in docstring

lib/Command/FixEncryptedVersion.php Outdated
@@ -204,7 +218,7 @@ private function correctEncryptedVersion($path, OutputInterface $output) {

//So decrementing did not work. Now lets increment. Max increment is till 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Max increment is till 5 is now outdated, isn't it?

@cdamken
Copy link

cdamken commented Oct 9, 2021

Why is this not merged? ;(

@DeepDiver1975
Copy link
Member

Why is this not merged? ;(

A hell of review comments have not been addressed by the developer 🤷

@IljaN IljaN force-pushed the add_option_for_max_increment branch from 1b2f862 to ce6c99e Compare October 11, 2021 11:48
@IljaN IljaN force-pushed the add_option_for_max_increment branch from ce6c99e to 097a4b3 Compare October 11, 2021 14:33
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 2 Code Smells

90.0% 90.0% Coverage
0.0% 0.0% Duplication

@IljaN IljaN merged commit 10a073d into master Oct 11, 2021
@delete-merged-branch delete-merged-branch bot deleted the add_option_for_max_increment branch October 11, 2021 16:36
@mmattel mmattel mentioned this pull request Oct 20, 2021
Closed
12 tasks
@IljaN IljaN mentioned this pull request May 23, 2022
Closed
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phil-davis phil-davis phil-davis left review comments

@mmattel mmattel mmattel approved these changes

@JammingBen JammingBen JammingBen approved these changes

Assignees

@IljaN IljaN

Labels
3 - To Review
Projects
None yet
Milestone
development
Development

Successfully merging this pull request may close these issues.

Add increment option to fix-encrypted-version
7 participants
@IljaN @mmattel @ownclouders @phil-davis @cdamken @DeepDiver1975 @JammingBen