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

Remove old ci file and add better dependency support #53

Merged
merged 2 commits into from
Nov 7, 2018

Conversation

alxdavids
Copy link
Contributor

  • Moved dependencies from dev to production.
  • Switched working directory to root
  • Using yarn install rather than npm command
  • New version of google-closure-compiler

Copy link
Member

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

Does the file addon/compiled/bg_compiled.js is autogenerated? If so, it must be excluded from the repo.
Otherwise, I considered is difficult to keep track on the changes of a minified source file.

Copy link
Member

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

Is it a strong requirement that packages listed on package.json correspond to that specific version?

@alxdavids
Copy link
Contributor Author

Does the file addon/compiled/bg_compiled.js is autogenerated? If so, it must be excluded from the repo.
Otherwise, I considered is difficult to keep track on the changes of a minified source file.

Yes it does and good point. I've removed the minified source files from the repo in the latest commit.

Is it a strong requirement that packages listed on package.json correspond to that specific version?

I guess not, but maybe having a fixed version is better as we are sure that it works in this state?

Copy link
Member

@armfazh armfazh left a comment

Choose a reason for hiding this comment

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

I agree with the changes.

@alxdavids alxdavids merged commit 65daca7 into master Nov 7, 2018
@alxdavids alxdavids mentioned this pull request Nov 7, 2018
@alxdavids alxdavids deleted the alxdavids/ci-build branch October 24, 2019 09:43
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