Skip to content

update GH pages deployment workflow example#1800

Open
andrewtchan wants to merge 1 commit into
11ty:mainfrom
andrewtchan:gh-pages-deployment-workflow-upgrade
Open

update GH pages deployment workflow example#1800
andrewtchan wants to merge 1 commit into
11ty:mainfrom
andrewtchan:gh-pages-deployment-workflow-upgrade

Conversation

@andrewtchan

Copy link
Copy Markdown

I used the GH pages deployment mini-tutorial workflow file last night to deploy my first 11ty project (yay). Noticed that it was slightly out of date from the current example file on the peaceiris repo.

https://github.com/peaceiris/actions-gh-pages#%EF%B8%8F-static-site-generators-with-nodejs.

@zachleat

Copy link
Copy Markdown
Member

hmm — I wonder if this makes a lot of assumptions about folks checking in package-lock files, to me it’s more likely that folks have a package.json checked in than a package-lock.json file

@andrewtchan

Copy link
Copy Markdown
Author

hmm — I wonder if this makes a lot of assumptions about folks checking in package-lock files, to me it’s more likely that folks have a package.json checked in than a package-lock.json file

Thanks for taking a look! I can't speak from experience to how likely those situations are, but strictly following the npm docs the recommendation is to check in package-lock.json and use npm ci for this use case. 1 2

I see the argument for simplicity and flexibility since this tutorial is aimed at beginners (like myself). So I do think using npm ci rather than npm install maybe assumes too much as it will cause an error in that case of package-lock.json not being checked in.

I don't see the point of caching package.json in any case since it's typically a very small file.

What do you think about keeping the cache action pointed at package-lock.json and using npm install instead of ci? Worst case is a cache miss (no error), average case (following npm's recommendations) is caching a much larger file. Alternatively, just remove the npm cache action entirely since it's not really the focus of the section above "Persisting Cache".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants