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

Using WarningsLibrary to query for warnings #6751

Merged
merged 5 commits into from
May 19, 2023
Merged

Using WarningsLibrary to query for warnings #6751

merged 5 commits into from
May 19, 2023

Conversation

JaroslavTulach
Copy link
Member

@JaroslavTulach JaroslavTulach commented May 18, 2023 •
edited
Loading

Pull Request Description

Fixes #6616 by using WarningsLibrary.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
  • All code has been tested:
    • Unit tests updated.

@JaroslavTulach JaroslavTulach self-assigned this May 18, 2023
@JaroslavTulach JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label May 18, 2023
@JaroslavTulach
Copy link
Member Author

obrazek

@JaroslavTulach JaroslavTulach requested a review from hubertp May 18, 2023 12:40
Akirathan
Akirathan approved these changes May 18, 2023
View reviewed changes
Copy link
Member

@Akirathan Akirathan left a comment

Choose a reason for hiding this comment

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

Good. Just minor nits. I hope the performance won't degrade by using uncached library? I guess we don't have any other option though.

engine/runtime/src/main/scala/org/enso/interpreter/instrument/job/ProgramExecutionSupport.scala Outdated Show resolved Hide resolved
@enso-bot enso-bot bot mentioned this pull request May 19, 2023
Closed
2 tasks
@JaroslavTulach
Copy link
Member Author

I hope the performance won't degrade by using uncached library? I guess we don't have any other option though.

This is not PE code. We have to use uncached version or we would need to do a more complicated rewrite.

@JaroslavTulach
Copy link
Member Author

JaroslavTulach commented May 19, 2023 •
edited
Loading

There is a failure in RuntimeServerTest - should the expected message just be updated, @4e6? The test seems to trigger the new code which then sends:

Api.ExpressionUpdate.Payload.Value(
    Some(
      Api.ExpressionUpdate.Payload.Value
        .Warnings(
          warningsCount,
          warning,
          WarningsLibrary.getUncached().isLimitReached(value.getValue)
        )
    )
)

even when the value.getValue is Vector - that's desirable change in behavior, I think. Updated in 123f65e

@JaroslavTulach JaroslavTulach added CI: Ready to merge This PR is eligible for automatic merge CI: Clean build required CI runners will be cleaned before and after this PR is built. labels May 19, 2023
@JaroslavTulach
Copy link
Member Author

Failure in RuntimeVisualizationsTest fixed in 224ae5c

@mergify mergify bot merged commit 995b3c9 into develop May 19, 2023
@mergify mergify bot deleted the wip/jtulach/WarningsLibrary_6616 branch May 19, 2023 13:39
Procrat added a commit that referenced this pull request May 22, 2023
@Procrat
Merge branch 'develop' into wip/procrat/fix-func-generation-on-projec…
0e9da81 
…t-rename

* develop:
  Widgets, Vector as Column, Viz Fixes and Rename Columns (#6768)
  Implement simple variants of `parse` for the Database backend (#6731)
  Enable `require-jsdoc` lint and add two lints related to React (#6403)
  Decimal/Integer .round and .int #6654 (#6743)
  Set suggestion reexports when serializing the library (#6778)
  Fix file uploading node expression. (#6689)
  Using WarningsLibrary to query for warnings (#6751)
  Implement `cast` for Table and Column (#6711)
  Display Initializing project... message when initializing project (#6661)
  Only send suggestions updates when type changes (#6755)
  sbt runEngineDistribution --debug to ease debuggging (#6745)
  Display "modified at" column on the cloud dashboard (#6687)
  Meta.meta Integer . methods (#6740)
  Show spinner while loading directory (#6714)
  Add cloud dashboard to changelog (#6688)
  Fix list editor panics during insertion (#6540)
  Update bug-report.yml
  Remove project create form (#6710)
  Change full-screen visualisation shortcut to shift-space (#6663)
Procrat added a commit that referenced this pull request May 22, 2023
@Procrat
Merge branch 'develop' into wip/procrat/home-button-6399
46418f1 
* develop: (30 commits)
  Widgets, Vector as Column, Viz Fixes and Rename Columns (#6768)
  Implement simple variants of `parse` for the Database backend (#6731)
  Enable `require-jsdoc` lint and add two lints related to React (#6403)
  Decimal/Integer .round and .int #6654 (#6743)
  Set suggestion reexports when serializing the library (#6778)
  Fix file uploading node expression. (#6689)
  Using WarningsLibrary to query for warnings (#6751)
  Implement `cast` for Table and Column (#6711)
  Display Initializing project... message when initializing project (#6661)
  Only send suggestions updates when type changes (#6755)
  sbt runEngineDistribution --debug to ease debuggging (#6745)
  Display "modified at" column on the cloud dashboard (#6687)
  Meta.meta Integer . methods (#6740)
  Show spinner while loading directory (#6714)
  Add cloud dashboard to changelog (#6688)
  Fix list editor panics during insertion (#6540)
  Update bug-report.yml
  Remove project create form (#6710)
  Change full-screen visualisation shortcut to shift-space (#6663)
  Revert "Show spinner when opening/creating a project (#6321)" (#6712)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Akirathan Akirathan Akirathan approved these changes

@hubertp hubertp hubertp approved these changes

@4e6 4e6 Awaiting requested review from 4e6 4e6 is a code owner

Assignees

@JaroslavTulach JaroslavTulach

Labels
CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

A simple vector with warnings does not highlight the node
3 participants
@JaroslavTulach @hubertp @Akirathan