peaceiris / actions-gh-pages

GitHub Actions for GitHub Pages 🚀 Deploy static files and publish your site easily. Static-Site-Generators-friendly.
https://github.com/marketplace/actions/github-pages-action
MIT License
4.71k stars 374 forks source link

feat: Add disable_nojekyll and cname options #112

Closed peaceiris closed 4 years ago

peaceiris commented 4 years ago

Yeah, I had exactly the same behavior in the Settings page.

Another problem I had additionally was that adding a empty commit after fixing the settings didn't seemed sufficient. It triggered the Gihub Actions workflow but not the Github Pages workflow. It seemed necessary to really alter the content of the gh-pages branch in order to publish. It may be useful that say that I'm using Parcel, which generates exactly the same content byte-per-byte given the same source files to optimize caches, and that I use a .nojekyll file to avoid problems.

I think Github has some mechanism to avoid publishing if the content of gh-pages didn't changed, at least when using the .nojekyll file.

By the way, this is completely unrelated but it could be a good idea to add that .nojekyll file by default in your action. 99% of the users will need it, will probably not read the whole documentation so they won't know, and will inevitably encounter a hard to debug problem sooner or later. (I'm convinced all serious users of Github Pages already lost at least one hour of their time because of that damn file like I did ^^ )

Originally posted by @nicolas-van in https://github.com/peaceiris/actions-gh-pages/issues/9#issuecomment-588129844

dhimmel commented 4 years ago

I am guessing this will be similar to the --no-jekyll option of ghp-import. The javascript gh-pages utility doesn't have this option, but does have a --dotfiles option that must be specified to copy over files starting with ..

Personally I like the ghp-import design where adding the .nojekyll file is an opt-in option.

peaceiris commented 4 years ago

Yes. Some static site generators like Hugo and Gatsby.js support to adding the .nojekyll file as a static asset. To avoid conflicting with that feature, supporting the add_nojekyll option may be better for us.

In addition, to enhance the option, we can put the explanation about add_nojekyll on the top of the README.

dhimmel commented 4 years ago

You might want to also consider an option to write a CNAME file for configuring custom domains. For this option, the user would want to pass a string that gets written to the CNAME file.

peaceiris commented 4 years ago

As I said at #25, the same may be said of other files (BingSiteAuth.xml, robots.txt, and so on). I think that it is better to manage those files as files.

What is the difference between (BingSiteAuth.xml, robots.txt) and (CNAME, .nojekyll)? Those all look just files. I do not know whether we should provide the add_nojekyll like feature, or not.

What do you think about this? @dhimmel @nicolas-van

dhimmel commented 4 years ago

What is the difference between (BingSiteAuth.xml, robots.txt) and (CNAME, .nojekyll)?

The difference is that CNAME and .nojekyll have special meaning for GitHub Pages. These files are essentially GitHub Pages settings, whereas the other files you mention are actual parts of the static website that would be relevant to other static hosts besides GitHub Pages.

I see your desire to keep things simple and don't have a strong feeling, but do think these options would be widely used if available. This action is designed for deploying to GitHub Pages.

nicolas-van commented 4 years ago

Personally I think that the fact that some static website generators support adding a .nojekyll is kind of a bad design from them (their job should just be to generate HTML). On the other hand it should be the job of a tool designed specifically to upload to Github Pages.

Also I think the Jekyll feature of Github as a whole should be considered completely deprecated since the availability of Github Actions. Why would you use a super specific feature when you can basically do whatever you want ? (Including triggering your own Jekyll build if you like that generator, but using an up to date version with the plugins you choose instead of a fixed set which has always been very limited.)

So my opinion regarding the .nojekyll file generation feature would be:

Regarding CNAME, now that we speak about it it doesn't seem like a bad idea to add an option to generate it through the custom action. I just think it could be cleaner to have something like that:

- name: Deploy
  uses: peaceiris/actions-gh-pages@v3
  with:
    deploy_key: ${{ secrets.ACTIONS_DEPLOY_KEY }}
    publish_dir: ./public
    cname: example.com

Thank like that:

- run: echo "example.com" > public/CNAME
- name: Deploy
  uses: peaceiris/actions-gh-pages@v3
  with:
    deploy_key: ${{ secrets.ACTIONS_DEPLOY_KEY }}
    publish_dir: ./public

Although I would say it doesn't seem very important, that's just personal taste.

dhimmel commented 4 years ago

I think the Jekyll feature of Github as a whole should be considered completely deprecated since the availability of Github Actions.

amen

To be an opt-out (only for the 0,1% of users that would like to use Github's custom Jekyll generator for legacy reasons)

While I agree the majority of users deploying to gh-pages probably want .nojekyll. However, I think this action has a lot of utility as a general purpose deploy-to-branch action. When deploying outputs to branches other than gh-pages, it could be confusing to users to have to add an option to prevent an extra file from being added.

peaceiris commented 4 years ago

Thanks all.

OK. I will add two options: disable_nojekyll and cname. Thanks to the opinions of you all, I got a clear reason to add those. I thought that we should keep this action's role which is just copying the assets until today, but the specific features of GitHub Pages seem to be necessary for us.

Why adding .nojekyll is default behavior?

While I agree the majority of users deploying to gh-pages probably want .nojekyll. However, I think this action has a lot of utility as a general purpose deploy-to-branch action. When deploying outputs to branches other than gh-pages, it could be confusing to users to have to add an option to prevent an extra file from being added.

I agree with this, so I am going to define the behavior like the following:

How about this spec?

nicolas-van commented 4 years ago

Seems good to me.

dhimmel commented 4 years ago

How about this spec?

Smart!

peaceiris commented 4 years ago

I opened #119 and released v3.3.0-0. Test it!

- name: Deploy
  uses: peaceiris/actions-gh-pages@v3.3.0-0
  with:
    deploy_key: ${{ secrets.ACTIONS_DEPLOY_KEY }}
    # github_token: ${{ secrets.GITHUB_TOKEN }}
    # personal_token: ${{ secrets.PERSONAL_TOKEN }}
    # publish_branch: master
    # publish_dir: ./public
    disable_nojekyll: true
    cname: peaceiris.com
peaceiris commented 4 years ago

https://github.com/peaceiris/peaceiris.github.io/runs/458272454?check_suite_focus=true#step:6:102

When the CNAME already exists, this action print the waring. Should we stop this action instead of printing the waring?

dhimmel commented 4 years ago

I don't think it matters too much, but the warning message could be clearer:

##[warning]CNAME already exists

Could this message be updated to indicate whether CNAME will be overwritten or not?

peaceiris commented 4 years ago

How about this?

##[warning]CNAME already exists, skip adding CNAME
peaceiris commented 4 years ago

119 has been merged, v3.3.0 released, and v3 updated.

Thank you all @nicolas-van and @dhimmel

peaceiris commented 4 years ago

You are co-author of #119 and added to the contributors.

peaceiris commented 4 years ago

The contributors do not have co-authors?

Screen Shot 2020-02-24 at 19 02 33

https://github.com/peaceiris/actions-gh-pages/commit/00fde1eb97e54d4fa89f31806ff2815878fafcb6

github-actions[bot] commented 3 years ago

This issue has been LOCKED because of it being resolved!

The issue has been fixed and is therefore considered resolved. If you still encounter this or it has changed, open a new issue instead of responding to solved ones.

Log | Bot Usage