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 manifest URL missing base #237

Merged
merged 10 commits into from
Oct 16, 2021

Conversation

j-maas
Copy link
Contributor

@j-maas j-maas commented Oct 2, 2021

No description provided.

@dillonkearns
Copy link
Owner

Thank you for the PR @j-maas!

It would be nice to get a test around this to make sure this continues to work in the future. Maybe we can add another diff test similar to the escaping example.

The way that test works is it just runs git diff --exit-code so it will give an error exit status if there are uncommitted changes. That way the committed dist output acts as an approved output that cannot change without re-approval.

The script is like this: https://github.com/j-maas/elm-pages/blob/863e0318a876a5b2166d28543b1de5561f40a544/examples/escaping/package.json#L7. If that sounds good, we can

@j-maas
Copy link
Contributor Author

j-maas commented Oct 3, 2021

Awesome test setup!

I noticed that you have tried to un-gitignore the .html files (https://github.com/dillonkearns/elm-pages/blob/master/examples/escaping/.gitignore#L4). It seems that this has not the desired effect.1 Instead, when you force-commit files, changes to them will actually be picked up without needing to add any rule to the .gitignore.

Is that something that you want me to apply to the other examples as well? That is, remove the obsolete .gitignore rule there?

Footnotes

  1. "It is not possible to re-include a file if a parent directory of that file is excluded." https://git-scm.com/docs/gitignore

@j-maas
Copy link
Contributor Author

j-maas commented Oct 3, 2021

Adding this example also demonstrates the issue that #234 solves: The index page is not rendered correctly.

<meta charset="UTF-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<script type="module">
import userInit from"\base-path\index.js";
Copy link
Owner

Choose a reason for hiding this comment

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

@j-maas is this something that is fixed by #236?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, yes! Didn't even notice it. 😅

@dillonkearns dillonkearns merged commit 3ece0dd into dillonkearns:master Oct 16, 2021
@dillonkearns
Copy link
Owner

Thanks so much @j-maas! 🙏

@j-maas
Copy link
Contributor Author

j-maas commented Oct 16, 2021

Thank you for helping me out with this! 😊

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