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

debt(encryption): Migrate app.php to Application.php #46136

Merged
merged 1 commit into from
Jul 1, 2024
Merged

debt(encryption): Migrate app.php to Application.php #46136

merged 1 commit into from
Jul 1, 2024

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Jun 26, 2024 •
edited
Loading

Needed to prevent exceptions generated from app.php to disable the encryption app.

⚠️ I haven't done any testing yet.

  • Check automatic tests
  • Do a quick smoke test

@artonge artonge requested review from julien-nc and kyteinsky June 26, 2024 12:52
@artonge artonge self-assigned this Jun 26, 2024
@artonge artonge added bug 2. developing Work in progress feature: encryption (server-side) php Pull requests that update Php code labels Jun 26, 2024
@artonge artonge added this to the Nextcloud 30 milestone Jun 26, 2024
@artonge artonge force-pushed the artonge/debt/encryption_application_boot branch 8 times, most recently from 6f19303 to c89087d Compare June 27, 2024 09:33
@artonge artonge added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 27, 2024
@artonge artonge requested a review from come-nc June 27, 2024 11:59
@artonge
Copy link
Contributor Author

artonge commented Jun 27, 2024

Seems to work in the current state.
But I would appreciate some reviews from more knowledgeable backenders.

@artonge artonge requested a review from icewind1991 June 27, 2024 12:00
come-nc
come-nc reviewed Jun 27, 2024
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

Two nitpicks, otherwise, awesome job, one less app.php 👍

apps/encryption/lib/AppInfo/Application.php Outdated Show resolved Hide resolved
apps/encryption/lib/AppInfo/Application.php Outdated Show resolved Hide resolved
@artonge artonge force-pushed the artonge/debt/encryption_application_boot branch from c89087d to 333ca35 Compare June 27, 2024 13:27
@artonge artonge requested a review from come-nc June 27, 2024 14:14
come-nc
come-nc approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

👍

@artonge
refactor(encryption): Migrate app.php to Application.php
39fd19f 
Signed-off-by: Louis Chemineau <louis@chmn.me>
@artonge artonge force-pushed the artonge/debt/encryption_application_boot branch from 333ca35 to 39fd19f Compare July 1, 2024 11:45
@artonge artonge merged commit 0166265 into master Jul 1, 2024
165 checks passed
@artonge artonge deleted the artonge/debt/encryption_application_boot branch July 1, 2024 15:08
This was referenced Jul 5, 2024
Open
Closed
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@kyteinsky kyteinsky kyteinsky approved these changes

@julien-nc julien-nc Awaiting requested review from julien-nc

@icewind1991 icewind1991 Awaiting requested review from icewind1991

Assignees

@artonge artonge

Labels
3. to review Waiting for reviews feature: encryption (server-side) php Pull requests that update Php code technical debt
Projects
None yet
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.
4 participants
@artonge @kyteinsky @come-nc @joshtrichards