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

Add a common-polyglot-core-utils project #5855

Merged
merged 6 commits into from
Mar 11, 2023

Conversation

radeusgd
Copy link
Member

@radeusgd radeusgd commented Mar 8, 2023

Pull Request Description

Adds a common project that allows sharing code between the runtime and std-bits.

Due to classpath separation and the way it is compiled, the classes will be duplicated - we will have one copy for the runtime classpath and another copy as a small JAR for Standard.Base library.

This is still much better than having the code duplicated - now at least we have a single source of truth for the shared implementations.

Due to the copying we should not expand this project too much, but I encourage to put here any methods that would otherwise require us to copy the code itself.

This may be a good place to put parts of the hashing logic to then allow sharing the logic between the runtime and the MultiValueKey in the Table library (cc: @Akirathan).

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@radeusgd radeusgd added the CI: No changelog needed Do not require a changelog entry for this PR. label Mar 8, 2023
@radeusgd radeusgd self-assigned this Mar 8, 2023
@radeusgd radeusgd force-pushed the wip/5583-database-column-names-review branch from 3b95fa0 to 04157df Compare March 8, 2023 13:22
@radeusgd radeusgd force-pushed the wip/radeusgd/common-polyglot-core-utils branch from d8d5709 to 349e767 Compare March 8, 2023 13:22
Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

Could you please add package-info.java to lib/scala/common-polyglot-core-utils/src/main/java/org/enso/polyglot/common_utils and copy some of the description in this PR there? Could be a useful future reference.

@radeusgd radeusgd force-pushed the wip/radeusgd/common-polyglot-core-utils branch from af2e4d3 to 1af3d14 Compare March 8, 2023 16:44
@radeusgd radeusgd force-pushed the wip/5583-database-column-names-review branch from 6c0fd58 to ac854e6 Compare March 8, 2023 20:27
@radeusgd radeusgd force-pushed the wip/radeusgd/common-polyglot-core-utils branch from 1af3d14 to 8e077e2 Compare March 8, 2023 20:27
@radeusgd radeusgd force-pushed the wip/5583-database-column-names-review branch from ac854e6 to 55f0611 Compare March 9, 2023 10:12
@radeusgd radeusgd force-pushed the wip/radeusgd/common-polyglot-core-utils branch 2 times, most recently from db6b824 to 89bf974 Compare March 9, 2023 10:14
@radeusgd radeusgd force-pushed the wip/5583-database-column-names-review branch from 3443b75 to cd99809 Compare March 10, 2023 08:31
Base automatically changed from wip/5583-database-column-names-review to develop March 10, 2023 19:08
@radeusgd radeusgd force-pushed the wip/radeusgd/common-polyglot-core-utils branch from 89bf974 to dbbf4ef Compare March 10, 2023 23:11
@jdunkerley jdunkerley added the CI: Ready to merge This PR is eligible for automatic merge label Mar 11, 2023
@mergify mergify bot merged commit 263c3ad into develop Mar 11, 2023
@mergify mergify bot deleted the wip/radeusgd/common-polyglot-core-utils branch March 11, 2023 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants