edgedb / easy-edgedb

An illustrated textbook designed to be a one-stop shop for learning EdgeDB
https://www.edgedb.com/easy-edgedb
69 stars 36 forks source link

Minor changes and issue resolution #93

Closed Dhghomon closed 1 year ago

Dhghomon commented 1 year ago

This PR contains fixes and changes to the lowest hanging fruit on the outstanding issues over the past months/years plus a bit of rewording and formatting in places. Not sure who's up for a review at the moment so will inform on Slack but @fantix please correct my Chinese where I added it!

Dhghomon commented 1 year ago

Awesome work on this! I noticed two things in particular I wanted to call out:

  • You've lower-cased a lot of the keywords, but not all of them. I think this would be the right time to just wholesale go through and change every instance of keywords to lower case so that the entire book is consistent with that new style. (I added suggestions for all the instances I found in the changes you made here. If you want to comb through now and try to get all the other instances changed, I'm happy to go behind you and do another check just to see if I find any stragglers.)
  • Sometimes, we render type names as inline code, and sometimes we render as plain text. I would lean toward always rendering them as inline code, but I'm OK with either as long as we're consistent.

The rest of my suggestions are relatively minor. Happy to discuss any of them.

This PR is going to be a very nice improvement to the book. I did a thorough review of the book a while back and have some other ideas for changes we might want to make, but those are the ones that stick out and are within the scope of this PR.

Thank you for going through this and taking care of a lot of the issues!

Good to confirm that lowercase is the norm now. I see a lot more of them now and suspected that was the way and have been changing them bit by bit. Agreed on the inline code for type names as well.

There are a lot more changes upcoming so feel free to let me know about your other ideas! The next major-ish change I think will be changing parts about migration to EdgeDB Projects syntax and output, and probably noting the schema changes at the end of each chapter which should be easy since the CLI prompts the user for each change. And with the .edgeql files out in the open now that'll probably mean talking about DDL a bit earlier on since it's become handier as a reference to remember how the schema has changed over time.

raddevon commented 1 year ago

There are a lot more changes upcoming so feel free to let me know about your other ideas! The next major-ish change I think will be changing parts about migration to EdgeDB Projects syntax and output, and probably noting the schema changes at the end of each chapter which should be easy since the CLI prompts the user for each change. And with the .edgeql files out in the open now that'll probably mean talking about DDL a bit earlier on since it's become handier as a reference to remember how the schema has changed over time.

Yes! Using the project CLI tools will be a great improvement, as well as the CLI migration tool. Is that something you're planning as well?

The biggest structural issue I noticed working through the book is that it seems there are examples shown in the book that the reader isn't intended to try. Then there are others that are meant to try. It doesn't clearly delineate which is which, so I found myself running a migration or inserting an object, only to find the book would later assume I hadn't done that. In some cases, I had to reverse it, but in others, I could just keep going, knowing that the output the book shows wouldn't always match my own, maybe because I had an additional object in my database or something like that.

I took some notes as I went through. I'd be happy to share them. I won't get my feelings hurt if you don't address everything, but maybe they will give you some ideas where else we might look to improve.

Dhghomon commented 1 year ago

There are a lot more changes upcoming so feel free to let me know about your other ideas! The next major-ish change I think will be changing parts about migration to EdgeDB Projects syntax and output, and probably noting the schema changes at the end of each chapter which should be easy since the CLI prompts the user for each change. And with the .edgeql files out in the open now that'll probably mean talking about DDL a bit earlier on since it's become handier as a reference to remember how the schema has changed over time.

Yes! Using the project CLI tools will be a great improvement, as well as the CLI migration tool. Is that something you're planning as well?

The biggest structural issue I noticed working through the book is that it seems there are examples shown in the book that the reader isn't intended to try. Then there are others that are meant to try. It doesn't clearly delineate which is which, so I found myself running a migration or inserting an object, only to find the book would later assume I hadn't done that. In some cases, I had to reverse it, but in others, I could just keep going, knowing that the output the book shows wouldn't always match my own, maybe because I had an additional object in my database or something like that.

I took some notes as I went through. I'd be happy to share them. I won't get my feelings hurt if you don't address everything, but maybe they will give you some ideas where else we might look to improve.

Yeah, that makes sense. Could maybe change the examples or have a symbol for ones that aren't necessarily part of the main path the reader is meant to follow.

I'll probably accept all your notes though! Since the goal is to make a full second edition of the book and the more new ideas and content the better. Definitely will change to include all the project tools. I think those parts might be separately delineated as well though so that Reader 1 (EdgeDB not installed, just learning) can just ignore them to concentrate on queries and whatnot, while Reader 2 (EdgeDB installed and ready to follow step by step) can do all the schema migrations and everything bit by bit as you did.

(Way further on there should be an interactive REPL like in the Tutorial, but probably do that waaay at the end. There's an issue suggesting that too)

raddevon commented 1 year ago

It would be great to try to integrate some of the 2.0 and 3.0 features that have launched since this first edition into a second edition too. It will be easy to pick those out in the documentation since they will be marked with a "New" label if you select the appropriate version in the version selection dropdown. I'm working on the 3.0 documentation and getting that rolled out piece by piece as we speak, so that can be a resource if you decide you would like to include it. Very exciting!

fantix commented 1 year ago

but @fantix please correct my Chinese where I added it!

Wow that's very cool! Looks generally fine but I think @daichen-daisy would want to make some small updates - though feel free to merge without them.

Dhghomon commented 1 year ago

Thanks! Nice to see I got it right.

daichen-daisy commented 1 year ago

@fantix sorry for the late reply.

@Dhghomon Wow!!! Amazing!!! Looks fantastic!!! I might have some minor updates about your work with the Chinese version. Can I submit a PR, or you prefer I leave some comments here?

Update: I have already created a PR.

Dhghomon commented 1 year ago

Thanks! Yep, PR looks like a good way to do it.