Skip to content

Conversation

@ntkme
Copy link
Contributor

@ntkme ntkme commented Dec 22, 2022

I observed is that lots of people install Jekyll 4.x locally with their own choices of dependency, test with that locally and check in the Gemfile. Then on GitHub it builds the site using Jekyll 3.x with a different set of dependency. The end results of this is very YMMV. It works sometimes, but in most cases won't. - E.g. #46

To some degree it is a user problem that they are using "jekyll" gem locally and the action runs "github-pages" gem, but this action is named "jekyll-build-pages" so that I think most of the users will get confused by what it actually does.

This PR prints a warning if we detect a Gemfile and found that some dependencies in the Gemfile cannot be met via "github-pages" gem preinstalled in the container, giving user some hints about what might be causing their build failure.

@ntkme ntkme requested a review from a team as a code owner December 22, 2022 01:26
@parkr
Copy link

parkr commented Dec 25, 2022

I like this idea generally. Could the user ever see it though if their build doesn't break?

This has been a common issue with Pages since forever since the Gemfile is always ignored.

@ntkme
Copy link
Contributor Author

ntkme commented Dec 25, 2022

I like this idea generally. Could the user ever see it though if their build doesn't break?

This should create an annotation for the run directly on the UI: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message

@yoannchaudet
Copy link
Contributor

yoannchaudet commented Dec 28, 2022

I like that too! Not everyone look at the warnings but they are visible enough we do get reports when our actions issue deprecation warnings.

Will look at the shellcheck issue tomorrow, that does not sound related to this PR.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants