Skip to content
This repository was archived by the owner on Jul 16, 2020. It is now read-only.

Conversation

@jorgevgut
Copy link
Contributor

Update readme file to include instructions on building ciao-webui
with the provided Makefile.

Remove instructions regarding install.sh script, as it is outdated.

Closes #246

Signed-off-by: Jorge Villalobos [email protected]

Update readme file to include instructions on building ciao-webui
with the provided Makefile.

Remove instructions regarding install.sh script, as it is outdated.

Signed-off-by: Jorge Villalobos <[email protected]>
In order to build application the 'install.sh' must be executed. This script will intsall dependencies using the npm package manager and also build minified JS scripts used with browser compatibility.
Note: this process is only necessary for development as minified scripts and dependencies are already provided by the application.
In order to build the application a Makefile is provided. Use make 'TARGET' to build Ciao-Webui. When running either 'install' or 'install-dev' targets. Note that if not set, the environment variable 'NODE_ENV' will be set to 'production' by default.

Choose a reason for hiding this comment

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

Are we planning to fix #129 any time soon? If not, we should add that npm install d3-scale needs to be run before make install-dev. Is this also needed for make install?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix is already in development branch.


In order to build application the 'install.sh' must be executed. This script will intsall dependencies using the npm package manager and also build minified JS scripts used with browser compatibility.
Note: this process is only necessary for development as minified scripts and dependencies are already provided by the application.
In order to build the application a Makefile is provided. Use make 'TARGET' to build Ciao-Webui. When running either 'install' or 'install-dev' targets. Note that if not set, the environment variable 'NODE_ENV' will be set to 'production' by default.

Choose a reason for hiding this comment

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

Use make 'TARGET' to build Ciao-Webui. When running either 'install' or 'install-dev' targets.

I'd reword slightly

Use make 'TARGET' to build Ciao-Webui, where TARGET can be either 'install' or 'install-dev'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, it's more clear.


###### make install
Use 'make' or 'make install' in order to build ciao-webui application with latest 'production' ready code.

Choose a reason for hiding this comment

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

I don't really understand the difference between make install and make install-dev from reading these two paragraphs.

  1. Does make install recompile the code?
  2. If it doesn't download the dependencies how does it build? Does it use a vendored version of the dependencies?
  3. What do you mean by 'production' here? Are you referring to the NODE_ENV?
  4. If do build using make install can I specify development when running deploy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @markdryan yes, the difference is that make install does not recompile js code, make install-dev does, I'll reword to make it more understandable. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it will be better to replace 'build' with 'install' as make install does not actually build any code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants