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

macro config parsing in core #319

Merged
merged 12 commits into from
Aug 3, 2023
Merged

macro config parsing in core #319

merged 12 commits into from
Aug 3, 2023

Conversation

C0W0
Copy link
Member

@C0W0 C0W0 commented Jul 26, 2023

Description

Currently, there is no easy way for macro developers to render custom configs to the front end. This PR contains Typescript parsing logic, which runs independently of the Deno runtime to extract custom config templates defined by the macro developer into SettingManifests. We might need some corresponding frontend logic to test it out.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Note: make sure your files are formatted with rust-analyzer

@netlify
Copy link

netlify bot commented Jul 26, 2023

Deploy Preview for lodestone-dashboard canceled.

Name Link
🔨 Latest commit 00443a7
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-dashboard/deploys/64c87cf5c1b96f0008bcf2ad

@C0W0 C0W0 changed the base branch from main to core_dev July 26, 2023 01:43
@netlify
Copy link

netlify bot commented Jul 26, 2023

Deploy Preview for lodestone-storybook canceled.

Name Link
🔨 Latest commit 00443a7
🔍 Latest deploy log https://app.netlify.com/sites/lodestone-storybook/deploys/64c87cf5699fed00083ffd0b

@C0W0 C0W0 force-pushed the core_macro_config_parsing branch from 6bc8037 to f2ab57e Compare July 26, 2023 01:58
@Ynng
Copy link
Member

Ynng commented Jul 27, 2023

I think this PR is directed at the wrong branch, core_dev and core_main are read only, we should merge to dev instead.

@Ynng Ynng requested a review from CheatCod July 27, 2023 00:53
@Ynng Ynng changed the base branch from core_dev to dev July 27, 2023 01:36
Copy link
Member

@CheatCod CheatCod left a comment

Choose a reason for hiding this comment

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

More comments on how everything works will be great

core/src/macro_executor.rs Outdated Show resolved Hide resolved
core/src/macro_executor.rs Outdated Show resolved Hide resolved
core/src/macro_executor.rs Outdated Show resolved Hide resolved
Copy link
Member

@CheatCod CheatCod left a comment

Choose a reason for hiding this comment

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

add test cases please

Copy link
Member

@CheatCod CheatCod left a comment

Choose a reason for hiding this comment

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

looks like some files are changed not related to this PR? you should revert them if that's the case

@C0W0
Copy link
Member Author

C0W0 commented Aug 1, 2023

@CheatCod added comments and fixed those irrelevant files. Just needed to do a merge with dev to get up to date. Also added unit test

@C0W0 C0W0 self-assigned this Aug 1, 2023
@C0W0 C0W0 merged commit 8e7d6bb into dev Aug 3, 2023
14 checks passed
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.

None yet

3 participants