Skip to content

Conversation

@zicsus
Copy link
Collaborator

@zicsus zicsus commented Apr 9, 2021

What does this PR do?

  1. Add platform specific properties to manifest. Now you can use chrome and firefox to define browser specific properties in manifest. Please note that chrome is used as default configuration.
  2. Copies whole assets folder to build dir except locales as they are processed separately.

Where to start reviewing?
gulpfile.js

Related Issues?
n/a

Caveats?
Builds file in same folder irrespective of build config. This can be an issue for parallel testing in Firefox and Chrome.

Reviewers?
@nkrusch

@nkrusch nkrusch merged commit 55579e6 into MobileFirstLLC:master Apr 9, 2021
@nkrusch
Copy link
Contributor

nkrusch commented Apr 9, 2021

@zicsus I have merged this but I have a concern about changing the assets output directory name because it will break any existing project that assumes a different directory name. Thoughts of how to address:

  • make major version increment or
  • make the directory name configurable
  • or can do both actually

@zicsus
Copy link
Collaborator Author

zicsus commented Apr 9, 2021

What do you mean different directory name?
Only thing that assets compiled was imgs, right?

@nkrusch
Copy link
Contributor

nkrusch commented Apr 9, 2021

Yes it compiles images only, but currently it does

assets/{images} -> dist/icons

after this change it will be

assets/{images} -> dist/assets

@nkrusch nkrusch added this to the 1.0.0 milestone Apr 10, 2021
@zicsus
Copy link
Collaborator Author

zicsus commented Apr 10, 2021

Major version increment is better step IMO because configuration will still break existing project. With major release we can lay down all the changes.

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.

2 participants