Skip to content

Conversation

@lhoward
Copy link
Contributor

@lhoward lhoward commented Nov 8, 2024

Fixes: #152

@lhoward lhoward force-pushed the lhoward/javac-version-args branch from 902fc47 to 8825d11 Compare November 8, 2024 12:02
@lhoward lhoward marked this pull request as ready for review November 8, 2024 12:12
/// Configuration for the Java2Swift translation tool, provided on a per-target
/// basis.
///
/// Note: there is a copy of this struct in the Java2Swift library. They
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we are forced into this because plugins can’t have dependencies, I wonder if we need to set up a ci task to check those are indeed the same… we can do that 🤔

But maybe later on, as the project becomes more stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, or maybe a makefile target to copy them from a SSOT.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I’d love to not have to use make in this build but I guess for such stuff maybe we will still… just because we already have swift gradle AND make so it’d be nice to just have two builds to worry about…

but i think it’d be okey to use make just for some simple automation but let actual “building” to swiftpm and gradle. So your idea here may be good :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Either way, right now the copy is fine

@ktoso
Copy link
Collaborator

ktoso commented Nov 8, 2024

Sorry for the delay, I'll get CI green tomorrow and merge all such pending PRs. Since we use nightly toolchain it regressed us accidentally

let configFile = URL(filePath: sourceDir)
.appending(path: "Java2Swift.config")
let configData = try Data(contentsOf: configFile)
let config = try JSONDecoder().decode(Configuration.self, from: configData)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, we probably want to make the config file optional for JavaCompiler. Let me fix this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense yeah

@lhoward lhoward force-pushed the lhoward/javac-version-args branch from 8825d11 to 0233236 Compare November 8, 2024 20:05
@ktoso ktoso merged commit 903b273 into swiftlang:main Nov 8, 2024
11 checks passed
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.

JavaCompiler: need flexible argument passing

2 participants