refactor(npm-scripts)

[debloper]

Addresses #2376
Also updates CI/CD pipeline to reflect the change.

• Breaks docker-compose & minishift orchestration. Fixed.
• Breaks run-planner.sh script's watch mode. Fixed

[debloper]

As part of this PR, I've updated semantic-release module and how it works now.
We were about 4 versions behind and completely on a different era of its CLI.

More details: semantic-release/semantic-release#480

[debloper]

This is how the updated docs on new npm scripts will look like.

[jarifibrahim]

@debloper npm run build doesn't work. The tests would obviously fail if planner doesn't start.
This is what I see after building planner via npm run build

[debloper]

@jarifibrahim: assuming you've done npm link on ../dist-watch instead of ../watch or ../dist.

This is why I'm consistently saying, the steps after functional tests on CI are more important for me. Considering putting the functional tests after the build step in CI - any objections?

[jarifibrahim]

@debloper No, I haven't done npm link ../dist-watch.

Considering putting the functional tests after the build step in CI - any objections?

The functional tests are already the last step in the CI build https://github.com/fabric8-ui/fabric8-planner/blob/master/deploy/release.groovy#L17
The issue here is that the files are not being copied in the correct directory.
npm build before/after the functional tests won't help in this case.

[debloper]

@jarifibrahim

No, I haven't done npm link ../dist-watch

Which directory did you link npm to from runtime then?

The functional tests are already the last step in the CI build

No, they come before fabric8-ui build.

And so that you're not misunderstanding my point, I want to use the build fail logs as the tool to identify and rectify the places where I need to change the code (you're suggesting how do I make it pass, or what needs change - which is what I'm aware of). A complete scripting overhaul patch with first push CI PASS isn't something I'm looking for, neither is that practical as well. Problem here is, I can't get to fabric8-ui build step without func-test terminating the CI.

[michaelkleinhenz]

Does this still break run_planner.sh?

[debloper]

@michaelkleinhenz yes it kinda does. First, here's the change:

From

# runs the planner in watch mode
function runPlanner {
  echo "Running Planner in $PLANNER_HOME"
  # as the watch task has to be started in the backgound, we need to create a minimal
  # dist-watch directory befor launching it otherwise the following linking gets confused.
  cd $PLANNER_HOME && mkdir -p dist-watch && cp package.json dist-watch/ && npm run watch:library &
} 

to

# runs the planner in watch mode
function runPlanner {
  echo "Running Planner in $PLANNER_HOME"
  cd $PLANNER_HOME && npm run build -- --watch &
} 

So, as mentioning in the inline comments, without a separate directory for watch mode, npm link gets confused - I don't know what for, not clarified.

Now, looking forward to standalone planner, we won't be doing npm link anymore anyway so this hack is no more required. Which means, ./dist-watch is no more required, and hence it's been removed in this patch. The generated build artifacts are kept in ./dist directory, no matter which build you're running (prod, dev, live-watch mode etc).

So, for the time being, watch mode is: broken, unless tested and confirmed that it's not.

Looking into this today, if the npm link confusion is something we can resolve easily.

[debloper]

@jarifibrahim @rgarg1

npm run build doesn't work. The tests would obviously fail if planner doesn't start.

Sure it does: https://jenkins.cd.test.fabric8.io/blue/organizations/jenkins/fabric8-ui%2Ffabric8-planner/detail/PR-2399/13/pipeline/

[fabric8cd]

@debloper fabric8/fabric8-ui:SNAPSHOT-PR-2399-30 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2399-fabric8-planner.badger.fabric8.io

[pranavgore09]

Hey @debloper looks a good path/restructure
I could talk about Jenkinsfile or release.groovy tough, would like to have review from others as it has JS and gulp changes @sanbornsen @nimishamukherjee
otherwise LGTM

[pranavgore09]

LGTM

[fabric8cd]

@debloper fabric8/fabric8-ui:SNAPSHOT-PR-2399-32 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2399-fabric8-planner.badger.fabric8.io

[michaelkleinhenz]

Could be that this conflicts with #2419 as I also modified some of the stuff there.

[debloper]

Could be that this conflicts with #2419 as I also modified some of the stuff there.

@michaelkleinhenz I don't think there's any functional conflict to happen for release.groovy. You might have to move a line or two up or down that is.

BTW, the build 33 failed with npm ERR! Invalid tar header. Maybe the tar is corrupted or it needs to be gunzipped? is one off random error or we did something new that needs changes? Ignore this, GKE goes nuts sometimes.

[fabric8cd]

@debloper fabric8/fabric8-ui:SNAPSHOT-PR-2399-36 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2399-fabric8-planner.badger.fabric8.io

[joshuawilson]

I think I had a issue with the newer version of semantic-release. But that was a while ago, don't remember what was the cause.

[debloper]

Must have been the pre/post steps, I'd guess, which aren't required anymore.
I had to refactor the release step a little bit to make it work with our build > release routines.