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

don't do extra steps in build so things are faster in the normal case #31

Merged
merged 2 commits into from
Feb 16, 2018
Merged

don't do extra steps in build so things are faster in the normal case #31

merged 2 commits into from
Feb 16, 2018

Conversation

jmazzitelli
Copy link
Collaborator

I propose we do not do anything extra in the build target. I'm removing both clean and format as dependent targets on build for the following reasons:

  1. format - we now have a pre-commit hook that will format the files for you on commit. Thus, there is no need to always format on build which is inefficient. The developer has options here - he can set up his IDE to always format on save (most devs do this) or they can "make format build" on the command line if they want to format during the build. By removing format dependency on build target, we don't force a format for EVERY build for EVERY developer.

  2. clean - if we clean on every build, it deletes the UI code we installed in _output/npm. This has the detrimental effect of always forcing the user to download again (via npm install) the UI code which takes a long time (in some cases half a minute, and when the UI code gets larger, that time will only grow). Thus to avoid having to npm install everytime a dev wants to do a code-deploy-test cycle, we should not perform a clean on every build. If a developer wants to perform a clean build, the option is available by simply running "make clean build".

In short, this PR is to allow the build and test cycle to be faster for the normal case.

Note this PR also ensures travis only has to build once.

Copy link
Member

@xeviknal xeviknal left a comment

Choose a reason for hiding this comment

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

LGTM!

@jshaughn
Copy link
Collaborator

LGTM, travis will clean so I don't have too!! (@jmazzitelli cookie for the ref)

@jshaughn jshaughn merged commit 0bac00d into kiali:master Feb 16, 2018
@jmazzitelli jmazzitelli deleted the fix-makefile branch February 16, 2018 14:13
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.

3 participants