Skip to content

ci: skip puppeteer chrome download#231

Merged
susnux merged 1 commit intomasterfrom
enh/skip-puppeteer-download
Oct 22, 2023
Merged

ci: skip puppeteer chrome download#231
susnux merged 1 commit intomasterfrom
enh/skip-puppeteer-download

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 <mail@danielkesselberg.de>
@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

max-nextcloud commented Oct 2, 2023

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 <max@nextcloud.com>
@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

nickvergessen commented Oct 4, 2023

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
@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

@susnux susnux susnux approved these changes

@skjnldsv skjnldsv skjnldsv 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