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

ci: skip puppeteer chrome download #231

Merged
merged 1 commit into from
Oct 22, 2023
Merged

ci: skip puppeteer chrome download #231

merged 1 commit into from
Oct 22, 2023

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Sep 30, 2023

Upstreaming nextcloud/server#40733

I'm uncertain if skip puppeteer download is a reasonable default for everyone.

Beside server only text has puppeteer as dependency.

@kesselb
ci: skip puppeteer chrome download
c7e97b8 
Signed-off-by: Daniel Kesselberg <[email protected]>
@kesselb kesselb self-assigned this Sep 30, 2023
@kesselb kesselb added 3. to review Waiting for reviews performance 🚀 Performances issues and optimisations labels Sep 30, 2023
@max-nextcloud
Copy link
Contributor

Beside server only text has puppeteer as dependency.

I did not know text even uses puppeteer and on closer inspection it's a single test from 2019 that does not run. Luckily it's in a subfolder that probably also does not trigger the download during npm runs.

So I'm unsure if it's worthwhile upstreaming this.

max-nextcloud added a commit to nextcloud/text that referenced this pull request Oct 2, 2023
@max-nextcloud
tests(frontend): cleanup old puppeteer based frontend test
a19a89f 
Superseeded by our cypress tests.

Only learned about it due to
nextcloud/.github#231

Signed-off-by: Max <[email protected]>
@max-nextcloud max-nextcloud mentioned this pull request Oct 2, 2023
Merged
2 tasks
@kesselb
Copy link
Contributor Author

kesselb commented Oct 2, 2023

So I'm unsure if it's worthwhile upstreaming this.

Good point.

The workflows in server are also updated (when someone triggers a global update) from here.
Is git able to merge the changes from here and still keep the skip puppeteer env in server?

@nickvergessen
Copy link
Member

Is git able to merge the changes from here and still keep the skip puppeteer env in server?

No, maintainer has to take care

@kesselb
Copy link
Contributor Author

kesselb commented Oct 4, 2023

No, maintainer has to take care

This means it's good to upstream it to avoid additional work in server while updating? ;)

susnux
susnux approved these changes Oct 22, 2023
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

👍

@susnux susnux merged commit 936b1e5 into master Oct 22, 2023
3 checks passed
@susnux susnux deleted the enh/skip-puppeteer-download branch October 22, 2023 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv approved these changes

@susnux susnux susnux approved these changes

@max-nextcloud max-nextcloud Awaiting requested review from max-nextcloud

Assignees

@kesselb kesselb

Labels
3. to review Waiting for reviews performance 🚀 Performances issues and optimisations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kesselb @max-nextcloud @nickvergessen @susnux @skjnldsv