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 0008: Builtin blocktypes #262

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Conversation

felix-oq
Copy link
Contributor

@felix-oq felix-oq commented Apr 12, 2023

@felix-oq felix-oq marked this pull request as draft April 12, 2023 09:51
Copy link
Member

@georg-schwarz georg-schwarz left a comment

Choose a reason for hiding this comment

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

Nice, looks really intuitive to me! Also hyped for the extension of this work towards composite blocks! ;-)

@felix-oq felix-oq marked this pull request as ready for review April 12, 2023 12:36
rfc/0008-builtin-blocktypes/README.md Outdated Show resolved Hide resolved
rfc/0008-builtin-blocktypes/README.md Show resolved Hide resolved
rfc/0008-builtin-blocktypes/README.md Show resolved Hide resolved
rfc/0008-builtin-blocktypes/README.md Outdated Show resolved Hide resolved
rfc/0008-builtin-blocktypes/README.md Show resolved Hide resolved
rfc/0008-builtin-blocktypes/README.md Show resolved Hide resolved
Copy link
Member

@georg-schwarz georg-schwarz left a comment

Choose a reason for hiding this comment

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

LGTM. We should probably look into the docs as well for the io-type case changes, although we don't have a real documentation therefore yet.

@georg-schwarz
Copy link
Member

Btw, I'm missing a mechanism to add documentation (description and examples) that we previously had in the BlockMetaInf mechanism. Any ideas how that would be approached?

@felix-oq
Copy link
Contributor Author

Btw, I'm missing a mechanism to add documentation (description and examples) that we previously had in the BlockMetaInf mechanism. Any ideas how that would be approached?

For now, I think it would be fine to leave that aspect in the langauge server (i.e. the language server "magically" provides documentation for builtin block types).

On the long run, we could use structured comments to describe documentation, similar to how it's done in programming languages. Recently I've noticed, that Langium offers some kind of JSDoc support, but I haven't looked into it so far: https://github.com/langium/langium/blob/main/packages/langium/CHANGELOG.md#jsdoc-support

@rhazn
Copy link
Contributor

rhazn commented Jun 23, 2023

We should merge this, right @georg-schwarz ? I can take over creating an implementation issue.

|-------------|----------------------|
| Feature Tag | `builtin-blocktypes` |
| Status | `DISCUSSION` | <!-- Possible values: DRAFT, DISCUSSION, ACCEPTED, REJECTED -->
| Responsible | `@felix-oq` |
Copy link
Member

Choose a reason for hiding this comment

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

We should probably change the responsible person.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done in the latest commit, I think then we can go like this, right? If we need to iterate on this I'd rather create a new PR.

@georg-schwarz georg-schwarz merged commit e3f4c5a into main Jun 23, 2023
2 checks passed
@georg-schwarz georg-schwarz deleted the rfc-0008-builtin-blocktypes branch June 23, 2023 11:56
@github-actions github-actions bot locked and limited conversation to collaborators Jun 23, 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