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

add OSX check for package #23

Merged
merged 3 commits into from
Jul 1, 2016
Merged

Conversation

shaikhjamir
Copy link
Contributor

While executing the package target added OS check

@@ -213,12 +222,14 @@ package() {
done; \
sed -i '' -e "s|http://localhost:8080|${server}|g" target/constants.json 2>/dev/null; \
sed -i '' -e "s|VERSIONLOC|${version}|g" target/app/index.html 2>/dev/null; \
if [ -z "$OS" ]; then \
(cd target; \
npm install; \
bower install; \
grunt clean; \
grunt build --target=develop --no-color); \
Copy link
Member

Choose a reason for hiding this comment

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

@longdogz in the context of jenkins, we still need to exec the above grunt build, no?

the objective here is to leverage ./bin/wasabi.sh to the fullest extent possible, which works quite well on osx but for linux we will assume (for now) the dependent toolchain we brew install today (eg vagrant, fpm, grunt, et al) is pre-installed, as it is in our internal system. as such, i suspect we need to exec the above non-install operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, @jwtodd , but I'm not sure what the context is. Are you asking what we should do related to the UI when doing a jenkins build internally? I guess it depends on whether you want to build and install the UI? If so, then you need those commands. It seems that "-z "$OS"" is a check if that var is empty, right? I don't see where it gets set in the script. But basically, those commands are necessary if you want the UI built at all.

Also, you mention something about grunt being pre-installed. I don't think it is pre-installed on any system, certainly, not on our laptops. It is on jenkins, because it was installed globally by hand (by me, probably). So, again, not sure what you're asking.

@jwtodd
Copy link
Member

jwtodd commented Jun 30, 2016

+1 from me ... but let's hear what @longdogz has to say

@coveralls
Copy link

Coverage Status

Coverage remained the same at 35.956% when pulling bc5847b on shaikhjamir:develop into f59a599 on intuit:develop.

@jwtodd jwtodd mentioned this pull request Jun 30, 2016
3 tasks
@jwtodd
Copy link
Member

jwtodd commented Jun 30, 2016

see: #15

2. Removed deb while creating rpm
3. Removed copy of feedbackserver from  modules/ui
@coveralls
Copy link

Coverage Status

Coverage increased (+27.0%) to 62.95% when pulling 4f23dfe on shaikhjamir:develop into f59a599 on intuit:develop.

@coveralls
Copy link

Coverage Status

Coverage increased (+27.0%) to 62.95% when pulling 0955019 on shaikhjamir:develop into f59a599 on intuit:develop.

@wasabi-honeybadger
Copy link
Collaborator

@shaikhjamir blocked on this one? if so, let's go ahead with the merge.

@wasabi-honeybadger
Copy link
Collaborator

@shaikhjamir :shipit:

@wasabi-honeybadger wasabi-honeybadger self-assigned this Jul 1, 2016
@wasabi-honeybadger wasabi-honeybadger merged commit ae26b02 into intuit:develop Jul 1, 2016
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.

None yet

5 participants