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

Initialize JS Regex crates and def AST. #1500

Merged
merged 7 commits into from
Nov 22, 2023
Merged

Conversation

ubugeeei
Copy link
Contributor

ref: #1164

I have initialized a crate for handling JavaScript Regexp and defined the AST.
I implemented the AST while referring to eslint-community/regexpp.

Review Points:

  • Is the crate name appropriate?
  • Are the crate settings appropriate? (I'm not very sure what should be done.)
  • Is the method of defining the AST correct? (Usage of Vex and Box)
  • Is the AST definition accurate? (It might be hard to tell if the information is sufficient without implementing a parser or visitor.)
    • Is the documentation appropriate?

This is my first PR for Rust code to OXC! Please check it out!
Thank you always for your kindness. It's really helpful. 🙏🏻

@ubugeeei ubugeeei marked this pull request as draft November 22, 2023 14:33
Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Re review points: yes to all questions!

Let's start merging after [lints] get added.

crates/oxc_js_regex/Cargo.toml Outdated Show resolved Hide resolved
crates/oxc_js_regex/Cargo.toml Outdated Show resolved Hide resolved
@ubugeeei ubugeeei marked this pull request as ready for review November 22, 2023 15:30
@ubugeeei ubugeeei mentioned this pull request Nov 22, 2023
4 tasks
@Boshen Boshen merged commit 8e923f1 into oxc-project:main Nov 22, 2023
18 checks passed
@Boshen
Copy link
Member

Boshen commented Nov 22, 2023

Nice work! You hands are getting Rusty ;-)

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.

3 participants