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 bug in assets generation #1269

Closed

Conversation

eksperimental
Copy link
Contributor

It would not pass the test in ExDoc.Formatter.HTMLTest named
"generates in default directory with redirect index.html file"

Since a macro was being used, if there were two assets at the compilation time,
after removing the leftover asset, it would still not pass the test because two assets
were in the compiled code.

It would not pass the test in ExDoc.Formatter.HTMLTest named
"generates in default directory with redirect index.html file"

Since a macro was being used, if there were two assets at the compilation time,
after removing the leftover asset, it would still not pass the test because two assets
were in the compiled code.
@josevalim
Copy link
Member

It Has to be a macro because ex_doc may be installed as an escript which we won’t have access to disk. So we need to embed their source in the compiled artifacts. Can you please explain how to reproduce the failure?

@eksperimental
Copy link
Contributor Author

It happens all the time when I run mix build, a new set of assets will be created. Even if I delete these new assets, the problem persist.

@josevalim
Copy link
Member

Sorry, it is still not clear what is the bug. How to reproduce it?

@eksperimental
Copy link
Contributor Author

mix clean

#test will pass
mix test

mix build

git restore formatters/epub/dist formatters/html/dist

# test will fail
mix test --force

#a way to solve it is to run `git clean` after `git restore`
git clean -f formatters/epub/dist formatters/html/dist
mix test --force

@eksperimental
Copy link
Contributor Author

and if the after running git clean you don't run mix test with the --force argument, it will make developers scratch their heads wondering how on earth it is still failing.

@eksperimental
Copy link
Contributor Author

I am thinking we could add a Mix task to deal with it. It is a common scenario since assets shouldn't be included in the PR.

@josevalim josevalim closed this in daf77c1 Sep 3, 2020
@eksperimental eksperimental deleted the fix_bug_assets branch September 3, 2020 13:55
@eksperimental
Copy link
Contributor Author

eksperimental commented Sep 3, 2020

@josevalim The problem is still there if you run the steps mentioned in #1269 (comment)

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

Successfully merging this pull request may close these issues.

None yet

2 participants