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

JBIDE-21422 - make server editor fields able to be edited for openshi… #1149

Merged
merged 1 commit into from May 11, 2016

Conversation

robstryker
Copy link
Member

…ft server

model.setInitializing(true);
try {
String conUrl = server.getAttribute(OpenShiftServerUtils.ATTR_CONNECTIONURL, (String)null);
ConnectionURL conurl2 = ConnectionURL.forURL(conUrl);
Copy link
Member

Choose a reason for hiding this comment

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

is it null safe?

Copy link
Member Author

Choose a reason for hiding this comment

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

no

@fbricon
Copy link
Member

fbricon commented May 9, 2016

Can you add some tests for the new code pretty please?

update(this.connection, connections, this.service, this.serviceItems);
}

public Connection getConnection() {
public IConnection getConnection() {
Copy link
Member

Choose a reason for hiding this comment

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

ServiceViewModelTest doesn't compile anymore

@fbricon
Copy link
Member

fbricon commented May 9, 2016

I have 2 connections to the cdk (admin/admin and openshift-dev/devel), but the connection combo doesn't select the openshift-dev one (blank instead)

@fbricon
Copy link
Member

fbricon commented May 9, 2016

Also the source path input text is unusable. I recommend the browse buttons to be put to the next line, aligned right.

@robstryker
Copy link
Member Author

Yes, I had mentioned in my standup that I need help with the connection not being pre-selected. UI issues on source path will be looked into.

@adietish
Copy link
Member

@robstryker ping me when you're on so that we can solve the connection selection issue.

@adietish
Copy link
Member

we discussed the issued and fixed the remaining flaw with the selection in the connection combo. I tested it quickly and it worked for me. The PR though still needs me to test it more intesively. Will do tomorrow.

@fbricon
Copy link
Member

fbricon commented May 10, 2016

It seems to work, except I can't set the route settings (seen in the advanced section of the wizard)

@robstryker
Copy link
Member Author

It seems to work, except I can't set the route settings (seen in the advanced section of the wizard)

I'd suggest this should be an enhancement and part of a separate jira. This jira is about making existing UI editable, and once we verify it works as expected we can isolate the route stuff separately.

@robstryker
Copy link
Member Author

Opened https://issues.jboss.org/browse/JBIDE-22327 to target that new issue.

@fbricon
Copy link
Member

fbricon commented May 10, 2016

Fair enough, so please rebase your PR so it can be applied.

@fbricon
Copy link
Member

fbricon commented May 11, 2016

+1 once rebased

…ft server

Update tests, make some utility methods private, fix bugs as per fred's comments

Small ui change in layout, removed some debugging code
@robstryker robstryker merged commit 2e13764 into jbosstools:master May 11, 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

3 participants