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

JBDS-4423 Fuse Tools installer integrated #751

Merged
merged 16 commits into from Jul 14, 2017

Conversation

dgolovin
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented Jun 16, 2017

Codecov Report

Merging #751 into master will increase coverage by 0.7%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #751     +/-   ##
=========================================
+ Coverage   97.92%   98.63%   +0.7%     
=========================================
  Files          31       31             
  Lines        2848     3010    +162     
  Branches      244      436    +192     
=========================================
+ Hits         2789     2969    +180     
+ Misses         59       35     -24     
- Partials        0        6      +6
Impacted Files Coverage Δ
browser/model/jbosseap.js 99.13% <ø> (+0.01%) ⬆️
browser/model/virtualbox.js 89.76% <ø> (+1.14%) ⬆️
browser/pages/confirm/controller.js 98.9% <100%> (+8.96%) ⬆️
browser/model/installable-item.js 100% <100%> (+1.53%) ⬆️
browser/services/componentLoader.js 100% <100%> (ø) ⬆️
browser/pages/start/controller.js 100% <100%> (ø) ⬆️
browser/model/jdk-install.js 97.29% <0%> (-2.14%) ⬇️
browser/model/helpers/downloader.js 96.94% <0%> (-1.52%) ⬇️
browser/model/devstudio.js 98.88% <0%> (-1.12%) ⬇️
browser/model/cdk.js 99.22% <0%> (-0.78%) ⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0b2931...0ffe44b. Read the comment docs.

@@ -9,14 +9,15 @@ import Logger from '../services/logger';
import JdkInstall from './jdk-install';

class DevstudioInstall extends InstallableItem {
constructor(installerDataSvc, targetFolderName, downloadUrl, fileName, devstudioSha256, additionalLocations, additionalIus) {
super(DevstudioInstall.KEY, downloadUrl, fileName, targetFolderName, installerDataSvc, true);
constructor(keyName, installerDataSvc, targetFolderName, downloadUrl, fileName, sha256sum, additionalLocations, additionalIus, useDownload) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any particular reason to use keyName in constructor here, while other model classes use a combination of static Class.KEY and this.keyName?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the reason is reusing it for fuse tools installation with additionalIus (IU - Installation Unit) configured in addition

@jrichter1
Copy link
Collaborator

I've run into a couple of problems while testing this:

  • fuse can be selected without devstudio (particularly deadly in online installer or if you did not bother with installing java)
  • fuse ignores bundled devstudio jar and always looks for it in the download folder, which is disastrous in the offline installer for a change

@jrichter1
Copy link
Collaborator

Or rather it might be caused by the fact that devstudio filename gets updated in requirements.json during the build and fuse doesn't.

@dgolovin
Copy link
Contributor Author

Thanks Jan, I am looking into both issues. First is related to the fact that we have no generic dependency resolution. I've added 'requires' attribute to every components where applicable, but fact that CDK should work with ether virtualbox or hyper-v holding. Hope I'll find solution and resolve this issue till my PTO starts.

Second seems to be simple bug to fix, because you already found the reason for it.

@dgolovin dgolovin changed the title JBDS-4423 Fuse installer integrated JBDS-4423 Fuse Tools installer integrated Jul 7, 2017
@dgolovin
Copy link
Contributor Author

dgolovin commented Jul 7, 2017

test this

Fix add baseOrder calulation based on requirements.json instead
of hardcoding it in componentLoader
This fix includes prep changes required for Fuse Tools Installer:
* moves installer ordering configuration from code to
requirements.json file;
* introduce additional parameter for devstudio installer to allow fuse
installer to provide correct key value;
* introduces additional parameters to list additional IUs to install;
* swithces to factory metod fromJason to create installer instances;
* add 'requires' attributes to installer's configuration, but those are
not used yet on confirmation page.
Fix add baseOrder calulation based on requirements.json instead
of hardcoding it in componentLoader
@dgolovin dgolovin merged commit 723438e into redhat-developer-tooling:master Jul 14, 2017
@dgolovin dgolovin deleted the fuse-installer branch January 30, 2018 14:09
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

3 participants