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

Simple refactoring of xuint classes #243

Closed
wants to merge 7 commits into from
Closed

Conversation

ggleyzer
Copy link
Collaborator

This is a refactoring suggestion for JK

// TODO: Add WorkerExecutor and the Gradle Worker API to execute in parallel if there are no dependencies.
// Any task with zero defined outputs is not cacheable, which should be enough for all run tasks.
// TODO: Make the module path/set pattern filterable for the module DSL.
public abstract class XtcTestTask extends XtcBaseRunTask implements XtcRuntimeExtension {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good. Let me know if you need help figuring out any DSL modifications, it should be relatively staightforward by looking at what is there already, but there are some hidden traps.

One thing that we want to do is to run the Gradle configuration cache, which will significantly speed up the pre-build phase of the gradle jobs - this requires that we do not store projects anywhere in the plugin, which is possible to do, but requires some rewrites. Enabling the configuration cache will tell us what goes wrong and why, and I was hoping to get a volunteer to this fairly self contained task.

javax-activation = { module = "com.sun.activation:javax.activation", version = "1.2.0" }
jakarta-xml-bind-api = { module = "jakarta.xml.bind:jakarta.xml.bind-api", version.ref = "jakarta" }
jaxb-runtime = { module = "org.glassfish.jaxb:jaxb-runtime", version.ref = "jakarta" }
jline = { module = "org.jline:jline", version.ref = "jline" }
mockito = {module = "org.mockito:mockito-core", version = "5.11.0"}

Copy link
Contributor

@lagergren lagergren Sep 24, 2024

Choose a reason for hiding this comment

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

Hitting ctrl-alt-L in intelliJ adds trailing after-brace spaces for me, so that I don't have to think too much about indentation, but I'm not sure if it's a personal setting or propagated to our codebase. We should work on IDE-environment consitency together with language support. I did add support for the Oracle-Sun Java code standard with checkstyle, but this seems to be a thing no one wants to migrate too, and is disabled. The Plugin java code does pass it, though. Personally, as long as something is consistent, it doesn't matter how it looks, I can read it. The Coherence standard is the only one I have had trouble doing that with, and it only partly has to do with brackets on single lines makes longer methods and frequent page ups / page downs with a laptop size screen. :-). We should likely implement a plugin for an official XTC code standard too, and only have one.

@thegridman
Copy link
Contributor

I'm closing this PR as the changes have been merged into PR #241

@thegridman thegridman closed this Sep 24, 2024
@ggleyzer ggleyzer deleted the gg/xtc-test-command branch September 25, 2024 16:26
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