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.

Sign up for GitHub

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

Jump to bottom

load versions for ignite extension wizard dynamically from file in this repo #1238

Merged
merged 1 commit into from Jan 17, 2018
Merged

load versions for ignite extension wizard dynamically from file in this repo #1238

merged 1 commit into from Jan 17, 2018

Conversation

lhein
Copy link
Contributor

@lhein lhein commented Jan 17, 2018 •
edited

Signed-off-by: Lars Heinemann lhein.smx@gmail.com

Pull Request Checklist

After this checklist is all checked or PR provides explanations for possible pass-through, please put the label "Ready for review" on the PR.

General

  • Did you use the Jira Issue number in the commit comments?
  • Did you set meaningful commit comments on each commit?
  • Did you sign off all commits for this PR? (git commit -s -m "jira issue number - your commit comment")

Functional

  • Did the CI job report a successful build?
  • Is the non-happy path working, too?

Maintainability

  • Are all Sonar reported issues fixed or can they be ignored?
  • Is there no duplicated code?
  • Are method-/class-/variable-names meaningful?

Tests

  • Are there unit-tests?
  • Are there integration tests (or at least a jira to tackle these)?

Legal

  • Have you used the correct file header copyright comment?
  • Have you used the correct year in the headers of new files?

@lhein
make ignite extensions versions dynamic
be67b52 
Signed-off-by: Lars Heinemann <lhein.smx@gmail.com>
@lhein lhein requested a review from a team as a code owner January 17, 2018 19:49
@lhein lhein merged commit ab38106 into jbosstools:master Jan 17, 2018
apupier
apupier reviewed Jan 18, 2018
View reviewed changes
...ension.core/src/org/fusesource/ide/syndesis/extensions/core/util/SyndesisExtensionsUtil.java

model.setCamelVersion(vMapping.getProperty(KEY_CAMEL_VERSION, "2.20.1"));
model.setSpringBootVersion(vMapping.getProperty(KEY_SPRING_BOOT_VERSION, "1.5.8.RELEASE"));
model.setSyndesisVersion(vMapping.getProperty(KEY_SYNDESIS_VERSION, "1.2.2"));
Copy link
Member

Choose a reason for hiding this comment

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

why 1.2.2? I thought that it was 1.2.0. Do you have a pointer for the 1.2.2 artefact?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because that will be most likely the minimum TP3 version number regarding Roland

Copy link
Member

Choose a reason for hiding this comment

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

https://mvnrepository.com/artifact/io.syndesis/syndesis-rest-parent/1.2.2 is available here.
There is also a 1.2.3 https://mvnrepository.com/artifact/io.syndesis/syndesis-rest-parent/1.2.3 but maybe built after Tech Preview 3. Where can we found the list of versions for Tech preview 3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

1.2.3 is the TP3 version. we need to adapt the version file.

apupier
apupier reviewed Jan 18, 2018
View reviewed changes
Copy link
Member

@apupier apupier left a comment

Choose a reason for hiding this comment

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

Am I understand it correctly, we have a single file so we can provide a version for a single version combination at a time?
As the 10.2 will work only with the TP3 and not the TP4, for TP4 we will need another files, right?

apupier
apupier reviewed Jan 18, 2018
View reviewed changes
...ension.core/src/org/fusesource/ide/syndesis/extensions/core/util/SyndesisExtensionsUtil.java
URL url = new URL(urlString);
vMapping.load(url.openStream());
} catch (IOException ex) {
// we ignore load errors
Copy link
Member

Choose a reason for hiding this comment

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

why ignoring them?
Might worth reporting them, it will be a notification that we removed the remote file although we shouldn't

Copy link
Contributor Author

Choose a reason for hiding this comment

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

method is used in a unit test. if we log the error via activator plugin log then we need to create integration test instead.

Copy link
Member

Choose a reason for hiding this comment

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

I think that it was the case "only because" the url used was on master and it was not merged yet.
I created https://issues.jboss.org/browse/FUSETOOLS-2768 to improve it

@lhein
Copy link
Contributor Author

lhein commented Jan 18, 2018

Am I understand it correctly, we have a single file so we can provide a version for a single version combination at a time? As the 10.2 will work only with the TP3 and not the TP4, for TP4 we will need another files, right?

That was really a last minute fix and the main purpose was to unblock users of the tp3. For tp4 we will rework that part including the wizard.

@apupier
Copy link
Member

apupier commented Jan 22, 2018 •
edited

Am I understand it correctly, we have a single file so we can provide a version for a single version combination at a time? As the 10.2 will work only with the TP3 and not the TP4, for TP4 we will need another files, right?

That was really a last minute fix and the main purpose was to unblock users of the tp3. For tp4 we will rework that part including the wizard.

@lhein
Do you have a JIRA already created for that? Or there is one on which adding these details?

@apupier
Copy link
Member

apupier commented Jan 29, 2018 •
edited

Am I understand it correctly, we have a single file so we can provide a version for a single version combination at a time? As the 10.2 will work only with the TP3 and not the TP4, for TP4 we will need another files, right?

That was really a last minute fix and the main purpose was to unblock users of the tp3. For tp4 we will rework that part including the wizard.

@lhein
Do you have a JIRA already created for that? Or there is one on which adding these details?

I created https://issues.jboss.org/browse/FUSETOOLS-2784

@apupier apupier mentioned this pull request Jan 29, 2018
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apupier apupier apupier left review comments

Assignees
No one assigned
Labels
PRIO READY FOR REVIEW
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@lhein @apupier