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

[RFC] Publish and Use: Elements and Libraries #476

Merged
merged 27 commits into from
Dec 6, 2023
Merged

Conversation

georg-schwarz
Copy link
Member

This PR is a reissue of #474.

Introduce concepts to enable multi-file Jayvee:

  • publish elements of a jv file for reuse in other files of the same project
  • libraries for bundling elements for reuse in other projects
  • use of elements and libraries in other files / projects

@georg-schwarz georg-schwarz marked this pull request as ready for review December 1, 2023 17:03
Copy link
Contributor

@rhazn rhazn left a comment

Choose a reason for hiding this comment

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

Looks good to me, I just left some pedantic grammar and expression comments.

My only comment on functionality is that I would remove the bracers around the use concept. I am fine with either decision.

rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
- Langium might not support this scoping mechanism out-of-the-box (more complex implementation)

## Alternatives

- "use" syntax without braces, etc., `use MyDomainLibrary1, MyDomainLibrary2 from './path/to/file.jv';`
- "use" syntax without braces, etc., `use MyDomainPackage1, MyDomainPackage2 from './path/to/file.jv';`
Copy link
Contributor

Choose a reason for hiding this comment

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

We kind of defaulted to this (I assume from TypeScript experience) but I actually agree. Why have bracers? I would remove them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I really like the braces.

Without it seems like some default exports (as a TS/JS-influenced dev) or different statements (int a = 0, b = 1). Braces clarify this imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

From a visual perspective, I also like the braces. Without braces, I'm unsure how very long use statements would be split into separate lines.

Copy link
Contributor

@joluj joluj left a comment

Choose a reason for hiding this comment

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

LGTM, just some minor comments but nothing that needs to be fixed.

rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
rfc/0015-multi-file-jayvee/0015-multi-file-jayvee.md Outdated Show resolved Hide resolved
- Langium might not support this scoping mechanism out-of-the-box (more complex implementation)

## Alternatives

- "use" syntax without braces, etc., `use MyDomainLibrary1, MyDomainLibrary2 from './path/to/file.jv';`
- "use" syntax without braces, etc., `use MyDomainPackage1, MyDomainPackage2 from './path/to/file.jv';`
Copy link
Contributor

Choose a reason for hiding this comment

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

I really like the braces.

Without it seems like some default exports (as a TS/JS-influenced dev) or different statements (int a = 0, b = 1). Braces clarify this imo.

@georg-schwarz
Copy link
Member Author

Thanks for your reviews - I think we designed something very useful here that will bring Jayvee one step closer to a productive DSL!

I incorporated the changes. Any last words before merging?

@rhazn
Copy link
Contributor

rhazn commented Dec 4, 2023

No, go for it, good job writing 😅👍.

@georg-schwarz georg-schwarz merged commit f01b2ad into main Dec 6, 2023
2 checks passed
@georg-schwarz georg-schwarz deleted the rfc-multi-file branch December 6, 2023 10:53
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants