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

Fix update when viewer is not installed #33457

Closed
wants to merge 1 commit into from
Closed

Fix update when viewer is not installed #33457

wants to merge 1 commit into from

Conversation

CarlSchwan
Copy link
Member

Since the viewer app is not shipped with server, currently with a fresh clone of
the server repo it is impossible to do an update.

@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Aug 4, 2022
@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Aug 4, 2022
@CarlSchwan CarlSchwan requested review from skjnldsv and a team August 4, 2022 08:55
@CarlSchwan CarlSchwan self-assigned this Aug 4, 2022
@CarlSchwan CarlSchwan requested review from PVince81 and come-nc and removed request for a team August 4, 2022 08:55
@PVince81
Copy link
Member

PVince81 commented Aug 4, 2022 •
edited
Loading

should we limit the check disablement for git clones ? this can be detected by checking if versions.php contains "git" as the update channel, there are some APIs to read it

@skjnldsv skjnldsv mentioned this pull request Aug 12, 2022
Merged
@PVince81
Copy link
Member

seems pef ci fails because of viewer, ex: https://github.com/nextcloud/server/runs/7844066536?check_suite_focus=true

@skjnldsv
Copy link
Member

server/core/shipped.json

Line 59 in 1c60ff5

"viewer",
maybe?

@CarlSchwan
Copy link
Member Author

server/core/shipped.json

Line 59 in 1c60ff5

"viewer",

maybe?

but then viewer won't be enabled by default anymore if I understand correctly :(

Also the ci is currently unhappy with my fix :(

@PVince81
Copy link
Member

@CarlSchwan adding to shipped.json is independent from "default_enable". most other shipped apps are also enabled by default, ex: user_status

@CarlSchwan
Fix update when viewer is not installed
fbecfa3 
Since the viewer app is not shipped with server, with a fresh clone of
the server repo it is imposible to do an update.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan CarlSchwan force-pushed the fix/update-nextcloud-without-viewer branch from 488fb21 to fbecfa3 Compare August 17, 2022 08:42
skjnldsv
skjnldsv requested changes Aug 17, 2022
View reviewed changes
Copy link
Member

@skjnldsv skjnldsv left a comment •
edited
Loading

Choose a reason for hiding this comment

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

AH, no, I meant that some logic behind alwaysEnabled might be at cause.
But we need to keep the alwaysEnabled on. Viewer can not be disabled as it will break tons of areas. We took the decision a while back.

@skjnldsv
Copy link
Member

@CarlSchwan adding to shipped.json is independent from "default_enable". most other shipped apps are also enabled by default, ex: user_status

The difference is that you can disable user_status without issues.

@skjnldsv skjnldsv mentioned this pull request Aug 18, 2022
Merged
@blizzz blizzz mentioned this pull request Aug 24, 2022
Merged
@CarlSchwan CarlSchwan closed this Aug 30, 2022
@skjnldsv skjnldsv deleted the fix/update-nextcloud-without-viewer branch August 30, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv requested changes

@PVince81 PVince81 Awaiting requested review from PVince81

@come-nc come-nc Awaiting requested review from come-nc

Assignees

@CarlSchwan CarlSchwan

Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 25
Development

Successfully merging this pull request may close these issues.
3 participants
@CarlSchwan @PVince81 @skjnldsv