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

Polishing Gradle HTML UI #1279

Merged
merged 6 commits into from
Jun 2, 2019
Merged

Polishing Gradle HTML UI #1279

merged 6 commits into from
Jun 2, 2019

Conversation

dukescript
Copy link
Contributor

@dukescript dukescript commented Jun 2, 2019

The HTML UI wizard needs some improvements to be considered stable.

  • people reported problems with app subproject - let's comment it out for forthcoming release
  • let's add a test to verify the generated project really builds - needs API for synchronous build execution
  • using "Run" doesn't work first time, only second time

CC @lkishalmi , @neilcsmith-net

@lkishalmi
Copy link
Contributor

"needs API for synchronous build execution" what doe that mean?

@@ -250,6 +251,9 @@ private static void invokeProjectAction(final Project project, final ActionMappi
prj.reloadProject(true, maxQualily, reloadArgs);
}
project.getLookup().lookup(AfterBuildActionHook.class).afterAction(action, outerCtx, task.result(), out1);
for (TaskListener l : context.lookupAll(TaskListener.class)) {
l.taskFinished(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.

This is the extension created for synchronous execution of the BUILD and RUN actions in the test, @lkishalmi.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I would not leak the task out of this context. As far as I see registering an AfterBuildActionHook could do the same as this task listener magic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rewritten to AfterBuildActionHook in cee89d4 - but it still needs a change in ActionProviderImpl. The previous code only searched project.getLookup()...

waiter.countDown();
};
actions.invokeAction(cmd, Lookups.fixed(prj, notifier));
waiter.await();
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 invokeCommand has to wait till the cmd action is finished, @lkishalmi.

@@ -250,6 +251,9 @@ private static void invokeProjectAction(final Project project, final ActionMappi
prj.reloadProject(true, maxQualily, reloadArgs);
}
project.getLookup().lookup(AfterBuildActionHook.class).afterAction(action, outerCtx, task.result(), out1);
for (TaskListener l : context.lookupAll(TaskListener.class)) {
l.taskFinished(t);
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I would not leak the task out of this context. As far as I see registering an AfterBuildActionHook could do the same as this task listener magic.

for (TaskListener l : context.lookupAll(TaskListener.class)) {
l.taskFinished(t);
for (AfterBuildActionHook l : context.lookupAll(AfterBuildActionHook.class)) {
l.afterAction(action, outerCtx, task.result(), out1);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

That's actually a nice enhancement. Thank you for that!

@asfgit asfgit merged commit 02028ea into apache:master Jun 2, 2019
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.

3 participants