Skip to content
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

Fix output folder #4

Merged
merged 3 commits into from
Aug 3, 2022
Merged

Conversation

askpatrickw
Copy link
Contributor

This fix uses the output folder defined in the config file.

@askpatrickw
Copy link
Contributor Author

But it is probably a breaking change for people...

Copy link
Owner

@rehanhaider rehanhaider left a comment

Choose a reason for hiding this comment

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

Would it not be better to parameterise the output folder? Something like

pelican ${PELICAN_CONTENT_FOLDER:=content} -o ${PELICAN_OUTPUT_FOLDER:=output} -s ${PELICAN_CONFIG_FILE:=publishconf.py}

After that we can add override paramets in action.yml. This will ensure people who are already using the action are not affected as well.

@rehanhaider
Copy link
Owner

Would it not be better to parameterise the output folder? Something like

pelican ${PELICAN_CONTENT_FOLDER:=content} -o ${PELICAN_OUTPUT_FOLDER:=output} -s ${PELICAN_CONFIG_FILE:=publishconf.py}

After that we can add override paramets in action.yml. This will ensure people who are already using the action are not affected as well.

Scratch that. I missed your comment about config file.

This will not be breaking change for existing users as the marketplace action is tagged to a particular version.

@rehanhaider rehanhaider merged commit 26da5de into rehanhaider:main Aug 3, 2022
@askpatrickw askpatrickw deleted the fix-output-folder branch September 22, 2022 03:08
bobobo80 added a commit to bobobo80/pelican-to-github-pages that referenced this pull request Feb 21, 2023
…folder"

This reverts commit 26da5de, reversing
changes made to 8339a6c.
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.

2 participants