Skip to content

Tools group and package#6

Merged
alexander-yevsyukov merged 14 commits into
masterfrom
tools-group-and-package
Jun 8, 2025
Merged

Tools group and package#6
alexander-yevsyukov merged 14 commits into
masterfrom
tools-group-and-package

Conversation

@alexander-yevsyukov
Copy link
Copy Markdown
Contributor

@alexander-yevsyukov alexander-yevsyukov commented Jun 6, 2025

This PR moves the compiler package under io.spine.tools, and allocates the Spine Compiler artifacts under the io.spine.tools Maven group.

Other notable changes

  • io.spine.annotation.Modified is used instead of javax.annotation.Generated because:
    1. Protobuf recently annotates its code with com.google.protobuf.Generated annotation.
    2. We do not generate the whole code, but modify it.
  • io.spine.tools.compiler.util.Format was deprecated in favor of io.spine.format.Format.
  • Gradle was bumped to 8.14.2.

@alexander-yevsyukov alexander-yevsyukov marked this pull request as ready for review June 6, 2025 21:04
@alexander-yevsyukov alexander-yevsyukov self-assigned this Jun 6, 2025
Copy link
Copy Markdown
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@alexander-yevsyukov LGTM in general.

There are a few things to address, though:

  1. Please update the README.md with the links to this repository. They are now pointing to ProtoData.

  2. There are several occasions of ProtoData still mentioned in the docs (cannot comment on the lines, as they aren't changed in this PR):

    • io.spine.tools.compiler.context.Member.kt,
    • io.spine.tools.compiler.ast.TypeExts.kt,
    • io.spine.tools.compiler.plugin.ConfigurationError,
    • io.spine.tools.compiler.render.Renderer.kt,
    • io.spine.tools.compiler.settings.LoadsSettings.kt,
    • io.spine.tools.compiler.settings.SettingsDirectory.kt,
    • io.spine.tools.compiler.settings.WithSettings.kt

And also, io.spine.tools.compiler.plugin.Policy.kt and io.spine.tools.compiler.plugin.View.kt reference the wrong Proto file via .../protodata/... path.

These I have found in Kotlin files. There are many more in .proto files which you can identify by searching for "ProtoData" in case-insensitive way.

@armiol
Copy link
Copy Markdown
Contributor

armiol commented Jun 7, 2025

@alexander-yevsyukov I only point out these things because I have initially encountered a wrong reference to the .proto file, and then searched for "ProtoData" to check if the problem is big. And there are really about 20-30 spots in which "ProtoData" or "/protodata/" should be changed to their respective counterparts.

We can do that in the following PR anyway.

@armiol
Copy link
Copy Markdown
Contributor

armiol commented Jun 8, 2025

@alexander-yevsyukov One more thing left here.

Copy link
Copy Markdown
Contributor

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@alexander-yevsyukov please see my comment.

Other than that, LGTM.

@alexander-yevsyukov alexander-yevsyukov merged commit b61dee0 into master Jun 8, 2025
6 checks passed
@alexander-yevsyukov alexander-yevsyukov deleted the tools-group-and-package branch June 8, 2025 11:20
@armiol
Copy link
Copy Markdown
Contributor

armiol commented Jun 8, 2025

@alexander-yevsyukov how come the PR merged automatically after it has been approved?

@armiol
Copy link
Copy Markdown
Contributor

armiol commented Jun 8, 2025

@alexander-yevsyukov FYI, I have just disabled this behavior.

image

Please take my comment into account next time.

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.

2 participants