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

Added require util. #5393

Merged
merged 1 commit into from
Apr 14, 2024
Merged

Added require util. #5393

merged 1 commit into from
Apr 14, 2024

Conversation

orizi
Copy link
Collaborator

@orizi orizi commented Apr 10, 2024

Used all around.


Stack:

⚠️ Part of a stack created by spr. Do not merge manually using the UI - doing so may have unexpected results.


This change is Reviewable

Copy link
Contributor

@gilbens-starkware gilbens-starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 27 of 28 files at r1.
Reviewable status: 27 of 28 files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware and @mkaput)

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

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

TBH, to my eyes this does not bring significant improvements, but I want to enforce my opinion here. I would love just not to use it in the LS

Reviewed 28 of 28 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware and @orizi)


crates/cairo-lang-language-server/src/lib.rs line 1218 at r1 (raw file):

    let syntax_db = db.upcast();
    let (node, lookup_items) = get_node_and_lookup_items(db, file, position)?;
    require(node.kind(syntax_db) == SyntaxKind::TokenIdentifier)?;

Could you please revert this change in the LS code? I would like to eventually minimize source dependencies on compiler utils and other crates in my areas of interest :/

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

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

*I do not want to enforce

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @ilyalesokhin-starkware and @orizi)

@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from 7a0b5d3 to 7b6cd4f Compare April 10, 2024 13:10
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 8ba6e06 to 248753f Compare April 10, 2024 13:10
Copy link
Collaborator Author

@orizi orizi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 27 of 28 files reviewed, 1 unresolved discussion (waiting on @gilbens-starkware, @ilyalesokhin-starkware, and @mkaput)


crates/cairo-lang-language-server/src/lib.rs line 1218 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

Could you please revert this change in the LS code? I would like to eventually minimize source dependencies on compiler utils and other crates in my areas of interest :/

Done.

Copy link
Member

@mkaput mkaput left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ilyalesokhin-starkware)

Copy link
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 27 of 28 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi)

@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 248753f to 266a217 Compare April 10, 2024 13:57
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch 2 times, most recently from 0228fb4 to 5806045 Compare April 10, 2024 14:10
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch 2 times, most recently from fb05129 to d9ec79e Compare April 10, 2024 21:56
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from 5806045 to 373a9bc Compare April 10, 2024 21:56
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from d9ec79e to 05fdb07 Compare April 11, 2024 06:22
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch 2 times, most recently from 044bde7 to d1ef438 Compare April 11, 2024 08:01
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 05fdb07 to 4fccb64 Compare April 11, 2024 08:01
@orizi orizi changed the base branch from pr/orizi/bounded-int/92bd8e52 to main April 11, 2024 08:06
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from c56c472 to 9459bb8 Compare April 13, 2024 08:08
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from bcc180f to 00a2ad3 Compare April 13, 2024 08:08
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 00a2ad3 to 3cd9dc5 Compare April 14, 2024 08:34
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch 2 times, most recently from 176e287 to d3730d8 Compare April 14, 2024 09:14
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 3cd9dc5 to 7ae1a03 Compare April 14, 2024 09:14
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from d3730d8 to c0819df Compare April 14, 2024 09:23
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 7ae1a03 to 55e53ce Compare April 14, 2024 09:23
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from c0819df to a95bc09 Compare April 14, 2024 09:29
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 55e53ce to 824c02e Compare April 14, 2024 09:29
Copy link
Contributor

@ilyalesokhin-starkware ilyalesokhin-starkware left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 28 files at r1, 2 of 3 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @orizi)

@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 824c02e to f4c531b Compare April 14, 2024 10:50
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch 2 times, most recently from 696f476 to 244296d Compare April 14, 2024 11:27
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from f4c531b to adb2930 Compare April 14, 2024 11:27
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from 244296d to b17a9d3 Compare April 14, 2024 11:44
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from adb2930 to 1476e34 Compare April 14, 2024 11:44
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from b17a9d3 to e6b19aa Compare April 14, 2024 11:45
@orizi orizi force-pushed the pr/orizi/bounded-int/92bd8e52 branch from 1476e34 to 53d8cad Compare April 14, 2024 11:45
Used all around.

commit-id:ec6469b5
@orizi orizi changed the base branch from pr/orizi/bounded-int/92bd8e52 to main April 14, 2024 12:09
@orizi orizi force-pushed the pr/orizi/bounded-int/ec6469b5 branch from e6b19aa to 50f0be8 Compare April 14, 2024 12:09
@orizi orizi enabled auto-merge April 14, 2024 12:09
@orizi orizi added this pull request to the merge queue Apr 14, 2024
Merged via the queue into main with commit 3748a58 Apr 14, 2024
84 of 85 checks passed
@orizi orizi deleted the pr/orizi/bounded-int/ec6469b5 branch April 17, 2024 16:23
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.

4 participants