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

Optional Project Name Prototype #864

Closed
wants to merge 1 commit into from

Conversation

osyrisrblx
Copy link

@Dekkonot
Copy link
Member

I had a bunch of thoughts typed up on this, but then I left them over the weekend and they are reduced to atoms. So instead I'll try to summarize.

I don't think this is the best way to handle this. I think we should make name optional on Project, then during the actual snapshotting process we can generate a name for it that will be overridden by the name field if it's filled.

This has a notable consequence due to ServeSession::project_name though. For that, we'll probably just want to verify that the top-level project (the one used during ServeSession::new) has a name and raise an error if it doesn't.

This is a bit more involved than the solution you have right now, so let me know if you want one of us to take over, but that's probably going to be the cleanest way to do this.

@osyrisrblx
Copy link
Author

osyrisrblx commented Feb 20, 2024

That makes sense to me 👍

This is a bit more involved than the solution you have right now, so let me know if you want one of us to take over, but that's probably going to be the cleanest way to do this.

Please do! I don't have a ton of time to invest into this right now and my lack of Rust experience doesn't help. 😆

I'm happy to help test of course.

@Dekkonot
Copy link
Member

Alright! I've implemented it and opened #870. Going to close this in favor of that.

@Dekkonot Dekkonot closed this Feb 20, 2024
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.

2 participants