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

Fixes a few bugs in parser (#292) #299

Merged
merged 3 commits into from
Jul 23, 2022
Merged

Fixes a few bugs in parser (#292) #299

merged 3 commits into from
Jul 23, 2022

Conversation

adams85
Copy link
Collaborator

@adams85 adams85 commented Jul 22, 2022

Aims to resolve #292. There were more problems around decorators than it looked at first. Hopefully, this would fix them all.

Copy link
Collaborator

@lahma lahma left a comment

Choose a reason for hiding this comment

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

Thanks for adressing also parsing bugs, help is really needed on this area. Looks good to my eye.

@lahma
Copy link
Collaborator

lahma commented Jul 23, 2022

Ready to merge?

@lahma
Copy link
Collaborator

lahma commented Jul 23, 2022

BTW, acorn source code is also a good reference for parsing some constructs, pretty neat code base.

@adams85
Copy link
Collaborator Author

adams85 commented Jul 23, 2022

Yeah, it's ready.

@lahma lahma merged commit 29f98aa into sebastienros:main Jul 23, 2022
@lahma
Copy link
Collaborator

lahma commented Jul 26, 2022

@adams85 hopefully you got an invite as collaborator to this repository and you can do a bit more without us blocking you.

If change looks OK to you and not too risky, act like you wish. PR peer review might be good for bigger changes but if people are unavailable, don't hesitate to use your own judgement.

@adams85
Copy link
Collaborator Author

adams85 commented Jul 26, 2022

I got the invitation, thanks for the confidence!

As a matter of fact, there are still a few things which would be nice to set straight for v3. Now it will definitely be smoother to make those changes.

@sebastienros
Copy link
Owner

Standard process is to not use a fork, just create local task branches, and when a PR is merged the branch is deleted (automatically). If you prefer forks feel free to continue though. I don't.

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.

A few slight parsing bugs
3 participants