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

Fix lifetime issues #58

Merged
merged 1 commit into from
Nov 14, 2022
Merged

Fix lifetime issues #58

merged 1 commit into from
Nov 14, 2022

Conversation

oblique
Copy link
Contributor

@oblique oblique commented Nov 5, 2022 •
edited
Loading

Fixes #57

@oblique oblique marked this pull request as draft November 5, 2022 02:00
@oblique
Copy link
Contributor Author

oblique commented Nov 5, 2022

For now this fixes #57, but I want to see if there are any other improvements that need to be done to helps as avoid any future mistakes.

@oblique oblique force-pushed the fix-lifetimes branch 5 times, most recently from dce1276 to 3e956fe Compare November 5, 2022 12:42
@oblique oblique marked this pull request as ready for review November 5, 2022 12:43
@oblique
Copy link
Contributor Author

oblique commented Nov 5, 2022

Ok it is ready for review

@oblique
Copy link
Contributor Author

oblique commented Nov 11, 2022

@ryan-summers ping

ryan-summers
ryan-summers reviewed Nov 12, 2022
View reviewed changes
Copy link
Collaborator

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

I don't have any functional concerns with the changes, just some architectural comments

tests/test.rs Outdated Show resolved Hide resolved
macros/src/codegen.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
@oblique oblique force-pushed the fix-lifetimes branch 3 times, most recently from f3bc176 to 780c110 Compare November 12, 2022 21:26
@oblique oblique force-pushed the fix-lifetimes branch 2 times, most recently from 12ca199 to 27d646c Compare November 13, 2022 01:04
@oblique
Copy link
Contributor Author

oblique commented Nov 13, 2022

All comments resolved. Please review again.

@oblique oblique force-pushed the fix-lifetimes branch 3 times, most recently from b749aa8 to 016c4cf Compare November 13, 2022 12:19
ryan-summers
ryan-summers reviewed Nov 13, 2022
View reviewed changes
macros/src/codegen.rs Outdated Show resolved Hide resolved
macros/src/parser/lifetimes.rs Outdated Show resolved Hide resolved
ryan-summers
ryan-summers approved these changes Nov 14, 2022
View reviewed changes
Copy link
Collaborator

@ryan-summers ryan-summers left a comment

Choose a reason for hiding this comment

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

Beautiful, thanks!

@ryan-summers ryan-summers merged commit 6566c9c into korken89:master Nov 14, 2022
@oblique oblique deleted the fix-lifetimes branch November 15, 2022 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryan-summers ryan-summers ryan-summers approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Multiple lifetime issues
2 participants
@oblique @ryan-summers