-
Notifications
You must be signed in to change notification settings - Fork 219
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
Bring back Git URL external access of the current project #1536
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -84,6 +84,7 @@ public ProjectScreenModel load() { | |
|
||
loadPOM(); | ||
loadKModule(); | ||
loadGitURL(); | ||
loadImports(); | ||
loadWhiteList(); | ||
loadRepositories(); | ||
|
@@ -103,6 +104,17 @@ private void loadKModule() { | |
model.setPathToKModule(project.getKModuleXMLPath()); | ||
} | ||
|
||
private void loadGitURL() { | ||
String value = ""; | ||
try { | ||
value = Paths.convert(project.getRootPath()).getFileSystem().toString(); | ||
} catch (final Exception ignore) { | ||
//this is basically for tests that don't use git file system | ||
} | ||
|
||
model.setGitUrl(value.replace("\n", " | ")); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean that Also it would be nice to have a unit test for this new loading behavior here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has always been the behavior of the jgit getFilesystem().toString(), here link for the code: https://github.com/kiegroup/appformer/blob/master/uberfire-nio2-backport/uberfire-nio2-impls/uberfire-nio2-jgit/src/main/java/org/uberfire/java/nio/fs/jgit/JGitFileSystemImpl.java#L104-L117 About the test: yes... ideally the test should cover this, but to make this happen this test has to be completely redone to use git not "file://" - and to do this it wouldn't be possible for the release happening today. I'd prefer to have this quick and dirty fix merged to be part of the community release and then follow up and fix the test. |
||
} | ||
|
||
private void loadImports() { | ||
model.setProjectImports(importsService.load(project.getImportsPath())); | ||
model.setProjectImportsMetaData(getMetadata(project.getImportsPath())); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By making this field read-only this PR is changing the semantics of the URL field in general settings.
Before this PR this field was connected to project pom.xml
<url>
element.In this PR this has changed to dynamically display URL of the workbench-managed repository.
This means that we should remove this outdated functionality from GeneralSettings presenter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the semantic of the URL was always intented to be the one present in this PR, use the pom data wasn't the original intentention.
ps: I was part of the UI design and personally asked UX to add this field to cover exactly the Git URL. This was overlooked during the merge.