Skip to content

chore: use correct prettier version in ci #8321

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.

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

Merged
merged 24 commits into from
Jul 5, 2023

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jul 5, 2023

Install globally is ignoring the yarn.lock with the right prettier version

The downside is installing prettier takes 1min each time 🤔

The caching thing

Let's get this green on main, then figure out how to get the prettier install to be quicker.

Add flag for node setup to allow not installing all node_modules
@Emyrk Emyrk force-pushed the stevenmasley/github_action_prettier branch from 277f0e9 to c46e06f Compare July 5, 2023 14:39
@Emyrk Emyrk changed the title chore: use local .lock prettier version chore: use same node setup for all ci jobs Jul 5, 2023
@@ -12,4 +17,4 @@ runs:
cache-dependency-path: "site/yarn.lock"
- name: Install node_modules
shell: bash
run: ./scripts/yarn_install.sh
run: ./scripts/yarn_install.sh ${{ inputs.yarn-install-flags }}
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to make sure this input is part of the cache key, otherwise we'll be hopping back-n-forth between prettier-only and full install?

Copy link
Member Author

@Emyrk Emyrk Jul 5, 2023

Choose a reason for hiding this comment

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

Maybe that is why the cache is always taking 1minute.

I wonder if I even need to run install.sh

@Emyrk Emyrk changed the title chore: use same node setup for all ci jobs chore: use correct prettier version in ci Jul 5, 2023
@@ -183,7 +183,7 @@ jobs:
run: "make --output-sync -j -B gen"

- name: Check for unstaged files
run: ./scripts/check_unstaged.sh
run: ./scripts/cgiheck_unstaged.sh
Copy link
Member

Choose a reason for hiding this comment

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

I guess it's an accident.

Copy link
Member Author

Choose a reason for hiding this comment

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

yup

@@ -203,7 +203,11 @@ jobs:
go-version: 1.20.5

- name: Install prettier
run: npm install -g prettier
run: cd site && yarn --cache-folder=/tmp/yarn_pretty_cache install --prettier
with:
Copy link
Member

Choose a reason for hiding this comment

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

this looks invalid,

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, I am just going to stop trying to use the cache in this PR. Let's get things green on main then mess around with making it faster. I'm new to github actions

Copy link
Member Author

Choose a reason for hiding this comment

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

It's blocking PRs.

@Emyrk Emyrk requested review from mafredri and matifali July 5, 2023 16:25
@Emyrk Emyrk merged commit d70e2d9 into main Jul 5, 2023
@Emyrk Emyrk deleted the stevenmasley/github_action_prettier branch July 5, 2023 17:11
@github-actions github-actions bot locked and limited conversation to collaborators Jul 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants