Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Regenerate manifest.json when config/manifest.js is changed #55

Merged
merged 1 commit into from
Jul 8, 2017

Conversation

simonihmig
Copy link
Contributor

Closes #48

@san650 Finally found some time to work on this. This is fully functional!

However I had to skip the broccoli test (node-tests/broccoli/generate-manifest-json-test.js) because I am not sure how to make this testable again. Or if this is actually needed...

This is because the Broccoli plugin now does much more, as it has to generate the manifest by itself, rather than getting the data from index.js. Because it has to regenerate it upon change, while the hooks in index.js are only invoked once. So currently it has access to the ember-cli project model, and with that dependency testing becomes harder. Would probably have to mock all relevant project methods to make this testable.

Not sure if that is really worth the needed effort, as the acceptance tests still cover the overall functionality. Your thoughts?

@simonihmig
Copy link
Contributor Author

@san650 ping :)

@san650
Copy link
Owner

san650 commented Jul 7, 2017

Hi @simonihmig let's delete the broccoli test, that's fine. As you said, acceptance test are already covering everything. :)

Do you need my help/assistance with anything else? More than glad to help. Let me know when you think this is ready to merge!

@simonihmig
Copy link
Contributor Author

@san650 deleted it (and replaced .jshintrc file for .eslintrc). This should be ready to be merged then. You might want to try this out before maybe, to make sure it works as expected. But it was for me...

@san650
Copy link
Owner

san650 commented Jul 7, 2017

Thanks @simonihmig

@san650
Copy link
Owner

san650 commented Jul 8, 2017

@simonihmig One of the problems I found with this PR (is not working well) is that this addon relies on another addon ember-web-app-rename to rename the manifest file -- to whatever name the user wants to use -- after the manifest is generated and processed by broccoli-asset-rev addon.

This process makes code reload hard because changing manifest.js file triggers the rebuild of ember-web-app but not the build of ember-web-app-rename 😞

Said that, this feature (renaming) was causing more problems than adding real benefits so I'm dropping it and plan to release a major version with this breaking change soon.

After this breaking change is released, I'll come back to this PR, update it to the latest version and merge it. Sorry for the delay and appreciate your help in making the addon better and better!

@simonihmig
Copy link
Contributor Author

@san650 Just rebased this and fixed the conflicts.

Indeed I missed the ember-web-app-rename thing. I tested this with the addon's dummy app, and a simple manifest.js I created for this. But this was not using the other rename addon. Now that the latter is gone, this should hopefully work as expected now!

@san650 san650 changed the title [WIP] Regenerate manifest.json when config/manifest.js is changed Regenerate manifest.json when config/manifest.js is changed Jul 8, 2017
@san650 san650 merged commit fb42d05 into san650:master Jul 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants