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

Include a pointer from the tutorial to a collection of examples #843

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

Pat-Lafon
Copy link
Contributor

Following up on #842 (comment)

@Pat-Lafon Pat-Lafon requested a review from yannham October 17, 2023 17:09
@Pat-Lafon Pat-Lafon mentioned this pull request Oct 17, 2023
@dburgener
Copy link
Contributor

I'm hesitant on this. The point of the doc content is for tutorials. The tutorial is telling you how to use lalrpop in your own project, including Cargo.toml changes. So to some extent, they are intended to function as examples for the users, rather than just maintaining our build system. To that end, referencing workspaces, both complicates things for the new(er) rust user, and is likely not relevant to the user's environment.

A counter-argument could be that version and edition are not at all the salient factors here, and users will be managing their own of those independent of lalrpop.

@yannham
Copy link
Contributor

yannham commented Oct 19, 2023

After taking a second look at this, I think I agree with @dburgener. I didn't realize those were example crates from the doc, and not proper subcrates of lalrpop. Also, I guess their version has no reason to be following LALRPOP (and nobol was indeed 0.1.0). Sorry for the misleading suggestion.

@dburgener
Copy link
Contributor

If we do abandon the workspacification, let's grab 72c2103 by itself though. That seems like a good change.

@youknowone
Copy link
Contributor

I agree that examples under doc is tutorial. I think we had similar discussion couples weeks ago. Leaving a comment on doc's Cargo.toml would be helpful

@Pat-Lafon
Copy link
Contributor Author

I guess I didn't remember the previous discussion on this. I think it still makes sense because none of these /doc/ examples are drop in as they are. They load lalrpop via a relative path instead of via crates.io. I viewed them more as examples of grammar files(since there is a quickstart tutorial for how to set up your crate for lalrpop).

I'm not too attached to this pr though so we can just cherry-pick the second commit to raise awareness to these examples.

@youknowone youknowone changed the title Workspace ify docs Include a pointer from the tutorial to a collection of examples Nov 17, 2023
@youknowone youknowone added this pull request to the merge queue Nov 17, 2023
Merged via the queue into lalrpop:master with commit c194d42 Nov 17, 2023
@Pat-Lafon Pat-Lafon deleted the workspace-ify_docs branch November 17, 2023 14:17
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