search

tschaub / gh-pages

General purpose task for publishing files to a gh-pages branch on GitHub
https://www.npmjs.com/package/gh-pages
MIT License
3.24k stars 190 forks source link

fix: Add missing cname option not passed to the config #535

Closed WillBAnders closed 10 months ago

WillBAnders commented 11 months ago

The --cname <CNAME> option added in #533 is not passed to the config, thus this option doesn't work in the current release. This includes that option and adds a test for the missing case.

Reviewing the original PR, the only other difference I see is that there's a nojekyll-exists spec but not a cname-exists spec - if there's a desire to have that as well I can add it.

paymand commented 10 months ago

@WillBAnders looks like you beat me to it. I'll close my PR.

Feel free to copy over the changes I made to logging here: https://github.com/tschaub/gh-pages/pull/536/files#diff-92bbac9a308cd5fcf9db165841f2d90ce981baddcb2b1e26cfff170929af3bd1

paymand commented 10 months ago

@WillBAnders also very good idea to add a test for cname-exists 👍

tschaub commented 10 months ago

Thanks for catching this and proposing a fix, @WillBAnders and @paymand. If you are able to add a regression test, that would be great. Otherwise I'll see if I can get to it.

WillBAnders commented 10 months ago

Tests added here should cover regression already. If you're referring to adding cname-exists as well; I'm all for that and should have time in the next few days.

WillBAnders commented 10 months ago

@tschaub Should be good from here. The added test asserts that --cname replaces the existing CNAME value; which matches expectations for how I would expect this to work (and existing behavior).

Hoping for a patch release following this being merged as well so I can close out another separate line of work. Thanks!

tschaub commented 10 months ago

Fix included in the 6.1.1 release. Thanks for the contribution, @WillBAnders.