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 UI element that displays model compiling status #911

Merged
merged 8 commits into from
Aug 5, 2022

Conversation

devanlooches
Copy link
Contributor

No description provided.

@devanlooches devanlooches marked this pull request as ready for review August 4, 2022 15:29
@devanlooches devanlooches changed the title Add UI element that displays status updates (Draft) Add UI element that displays status updates Aug 4, 2022
@devanlooches devanlooches changed the title Add UI element that displays status updates Add UI element that displays model compiling status Aug 4, 2022
@hannobraun hannobraun self-requested a review August 5, 2022 06:31
Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you, @devanlooches!

This looks almost good to merge now, but the CI build still fails, specifically the cargo fmt and cargo clippy steps.

You should be able to take care of formatting automatically by running cargo fmt and just committing the result. You can fix the Clippy error by running cargo clippy and following its suggestion.

To get a feel for whether the CI build will pass, you can run just build locally, as documented here: https://github.com/hannobraun/Fornjot/blob/main/CONTRIBUTING.md#making-improvements

@devanlooches
Copy link
Contributor Author

devanlooches commented Aug 5, 2022

For some reason the CI build passes. However locally when I ran just build it failed with a linker error on the export validator step.

@devanlooches devanlooches requested a review from hannobraun August 5, 2022 14:36
Copy link
Owner

@hannobraun hannobraun left a comment

Choose a reason for hiding this comment

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

Thank you, @devanlooches, looks good!

@hannobraun hannobraun enabled auto-merge August 5, 2022 15:13
@hannobraun hannobraun merged commit 2a0c688 into hannobraun:main Aug 5, 2022
@hannobraun
Copy link
Owner

Oh, I forgot to reply to this:

However locally when I ran just build it failed with a linker error on the export validator step.

That's sounds like a bug! Could you open an issue with the full error message? Maybe it's something platform-specific, or I have something installed that's missing on your machine.

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.

2 participants