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.

Sign up for GitHub

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

Jump to bottom

Launcher Native Image Build #970

Merged
merged 25 commits into from
Jul 10, 2020
Merged

Launcher Native Image Build #970

merged 25 commits into from
Jul 10, 2020

Conversation

radeusgd
Copy link
Member

@radeusgd radeusgd commented Jul 6, 2020 •
edited
Loading

Pull Request Description

Closes #973

Important Notes

Adds launcher builds to the built artifacts on the CI.
Renames enso-<version> artifact to enso-engine-<version> to be consistent with naming of the planned release structure.

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 where possible.

@radeusgd radeusgd added Type: Enhancement -tooling Category: tooling --breaking Important: a change that will break a public API or user-facing behaviour p-high Should be completed in the next sprint labels Jul 6, 2020
@radeusgd radeusgd self-assigned this Jul 6, 2020
@radeusgd radeusgd force-pushed the wip/rw/launcher-native-image branch from 601823f to 8835c36 Compare July 6, 2020 14:33
@iamrecursion iamrecursion changed the title [WIP] Launcher Native Image build [WIP] Launcher Native Image Build Jul 6, 2020
@radeusgd radeusgd force-pushed the wip/rw/launcher-native-image branch 8 times, most recently from b2a7f99 to c3e1993 Compare July 8, 2020 13:59
@radeusgd radeusgd changed the title [WIP] Launcher Native Image Build Launcher Native Image Build Jul 8, 2020
@radeusgd radeusgd marked this pull request as ready for review July 8, 2020 14:01
@radeusgd radeusgd force-pushed the wip/rw/launcher-native-image branch 3 times, most recently from b6e4858 to 9dac1db Compare July 8, 2020 16:27
kustosz
kustosz suggested changes Jul 8, 2020
View reviewed changes
Copy link
Contributor

@kustosz kustosz left a comment

Choose a reason for hiding this comment

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

Too much copy paste to accept

distribution/launcher/NOTICE
@@ -0,0 +1,112 @@
Enso
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Who created this file?
  2. What's the future recipe for updating it?
  3. Is it documented?
  4. Is this list exhaustive?
  5. How do you know this is enough to satisfy these licenses?

Copy link
Member Author

Choose a reason for hiding this comment

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

1 - I have added it by concatenating NOTICEs based on the licenses of the projects that we (may) currently include in the build.
2, 3 - I did miss that - I will add documentation specifying that it should be updated.
4. It should include all (transitive) dependencies that are used in the launcher (for now there aren't many).
5. I have spent a fair amount of time reading them, but indeed I am not sure and would rather get a confirmation from someone who has better knowledge of legal affairs.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. Document what currently is here. After this PR lands, open a task for legal review of these, assign @sylwiabr and @iamrecursion

Copy link
Member Author

Choose a reason for hiding this comment

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

I have documented the current process at https://github.com/enso-org/enso/blob/9a6f41e21932ebf3ef6eabc8b0c3bc21dcda4882/docs/distribution/licences.md

I will open the mentioned task after the PR is merged.

@radeusgd radeusgd force-pushed the wip/rw/launcher-native-image branch from 9dac1db to 3ac1b16 Compare July 9, 2020 11:10
@radeusgd radeusgd requested a review from kustosz July 9, 2020 14:18
@radeusgd radeusgd force-pushed the wip/rw/launcher-native-image branch from 9a6f41e to 91b7c0b Compare July 9, 2020 14:21
kustosz
kustosz approved these changes Jul 9, 2020
View reviewed changes
Copy link
Contributor

@kustosz kustosz left a comment

Choose a reason for hiding this comment

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

Beautiful!

@radeusgd radeusgd merged commit 610a38d into main Jul 10, 2020
@radeusgd radeusgd deleted the wip/rw/launcher-native-image branch July 10, 2020 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@4e6 4e6 4e6 approved these changes

@kustosz kustosz kustosz approved these changes

@iamrecursion iamrecursion Awaiting requested review from iamrecursion

@lolczak lolczak Awaiting requested review from lolczak

Assignees

@radeusgd radeusgd

Labels
--breaking Important: a change that will break a public API or user-facing behaviour -tooling Category: tooling p-high Should be completed in the next sprint
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Set Up CI for the Launcher with Native Image Build
3 participants
@radeusgd @4e6 @kustosz