Open mattwelke opened 6 years ago
Thanks @welkie! FYI, I'm currently super busy helping organize a conference in SF (https://iterateconf.io), so I won't be able to take a look right away. I'm excited to get contributions though! 😄
@nbarbettini Sounds good. If you don't mind me bugging you for some advice though in the mean time, I was going to contribute another chapter or fix a few up according to the issues that we've got logged right now. But I'm new to submitting pull requests to other projects, so I'm not sure how to go about doing that. If I added to this pull request, it wouldn't be ideal because accepting all changes would be all or nothing. But if I do new branches for each addition I do, with individual pull requests, then the 2nd 3rd etc wouldn't be able to be merged after the 1st is merged (I'd have to manually update the branches for them with the latest code now that the 1st was merged). Unless my understanding of that isn't good? I'd appreciate some advice if you know the best way to do contributions like that.
Your link goes to a 404 page. Please kindly check it again. Thank you!
@sadqiang GitBook unfortunately disabled my account after I created that book. They classified it as spam. I think they got confused assuming I was intending to commit copyright infringement or something. Unfortunately, we can't see my changes rendered unless we know how to simulate GitBook locally or the pull request is accepted. You can see the changes by checking the "Files changed" in this pull request though.
@welkie Good question - I'd definitely prefer to keep pull requests separate instead of adding to one giant PR.
If you create a new branch other-changes
off master
and make edits that don't depend on anything in this branch, then other-changes
can be PRed and merged to master
independently of this one. You won't have to wait for this PR to be merged.
Also, you can always merge or rebase the latest changes from master
into a branch you are working on to keep it "fresh". My typical process for working on a branch is:
git checkout -b other-changes
master
when it's ready for reviewmaster
so everything in the branch sits on top of the latest commits to master
Hopefully I understood your question and scenario. If you need some help or just want to see how it works, shoot me an email and we can do a quick screenshare call.
@welkie @sadqiang It is possible to use Gitbook locally: https://toolchain.gitbook.com/setup.html
That's super weird that Gitbook disabled your account. Other forks of the book have been hosted on Gitbook just fine. /shrug
@nbarbettini That's basically how I planned to do it. Separate branches each with PRs. I didn't realize you could submit PRs for merging those branches into the origin repo's master. That helps and should keep the work for the origin repo's maintainers limited. They would just need to accept the individual PRs which probably won't conflict. I think that clears it up for me. I'll leave you alone to prepare for Iterate. Good luck! :+1:
Reviewed. I like it, thanks for the contribution! The tone fits the book well.
The only thing I'm still thinking through is
For sure. I'm thinking it's worth at least mentioning MongoDB, even though its provider is very beta right now. MySQL also has pretty good support.
I'll touch up the minor things from the review soon. In the mean time, if you have an idea for the best place chronologically for this topic, be sure to let me know.
@nbarbettini I figured I'd look into cleaning this up now.
I was thinking about this point you made:
Where it should go chronologically. It currently feels a little weird to explore this after the application is deployed.
I figured what might be nice would be having it after the "Deploy to Azure" chapter, in a new chapter called "Deploy to Heroku". Heroku doesn't support SQLite, so this would be a good chance to show readers two new concepts:
It gives PostgreSQL more relevance as opposed to it being randomly recommended like my existing chapter seems to do, since PostgreSQL is the simplest RDBMS to use on Heroku.
Alternatively, I was also thinking it would make sense to have the database switched to PostgreSQL before the deploy to Azure. This would also give it a relevant role, showing users that SQLite usually only makes sense in development, and that we would use something heftier in production.
I left the arrangement alone for now, but cleaned it up using your feedback. You can see the changes in https://github.com/nbarbettini/little-aspnetcore-book/pull/49/commits/817bbf6be05ea8e2a6e6dc0804a988454d9106f8.
@welkie I really like the idea of a Deploy to Heroku chapter! Would you be willing to write that?
This logical progression makes the most sense to me so far:
That sounds good to me! I played around with the Heroku buildpack before and I noticed some rough edges. Not sure if it was me or the buildpack itself. I'm going to look more into it and if I find that it works smoothly, I'll make that new chapter and merge the PostgreSQL content in.
I'm prepping for exams right now, so that will be something I get started with later.
@nbarbettini I was able to get my Heroku deploy working.
This thread details what I had to do to get around the inability to run migrations via the Heroky CLI right now (at this point I'm not sure if that's a bug with the buildpack or if that's just me being dumb).
I don't think it's that bad actually now that I've done it. I think it can be explained simply in a Heroku chapter. Let me know what you think of that thread and if you give me the go ahead, I'll work on the Heroku chapter shortly and integrate my PostgreSQL stuff into it.
If the author of the buildpack doesn't get back to you soon, I think it makes sense to go ahead and doc the workaround. 👍
FYI I'm releasing version 1.1 today and planning on releasing another large update around the time that ASP.NET Core 2.1 is final. The new stuff we've been discussing here fits in well with the next update I think. 👍
@nbarbettini I agree. I've been busy lately starting my first post grad job so this hasn't been on my radar. It will be soon though. :)
Wanted to provide an update here. I was coordinating with the maintainers behind the .NET Core buildpack to find out the best way to go about deploying .NET Core to Heroku. There ended up being some disagreement on the best way to handle db migrations. There were two solutions found:
<Exec Command="dotnet ef database update" />
) to run the SDK during the deploy step to run pending migrations.And another user put forth an idea, to use the official Heroku "Release" system to do migrations.
I was waiting for them to sort out what they think would be the best option but it hasn't gone anywhere and I'd like to finish the PR since I sent it in February and I think it would be useful for users. I'd welcome some input here and then I can finish the chapter with how we'll instruct our readers to deploy on Heroku.
@nbarbettini
Rebased and redid the chapter to be a new "Deploy to Heroku with PostgreSQL" chapter. The debate on the best way to handle Heroku side migrations was never solved, so I decided to go with the solution where the source code is changed to run pending migrations automatically as the application starts up if it detects that it's running on Heroku. You can take a look at the commits to see this. Despite 2.1 having been released, which includes some slight differences in how the Program.cs
code would look, I kept this based on 2.0, since we can tackle the 2.1 update later on.
@nbarbettini Was wondering if you've had a chance to look at this? Also, one thing I was thinking was that this might be too much for this book, since its goal is to be focused on just introducing ASP.NET Core to people, and it already covers one deployment method. Curious what your thoughts are.
Hope my first contribution with new content looks good! I tried to follow the style and tone of the book as best as I could (friendly, conversational, but still short and to the point).
Every step I instruct the reader to do in the book is assuming they've completed the book so far at least to the point where they add the
TodoItem
model. I also tested each step as I went to make sure it worked properly, and it worked on my machine (Windows 10, PostgreSQL 10.2 via Docker).I was able to get my own fork of the book hosted (https://welkie.gitbooks.io/littleaspnetcorebookfork/content/chapters/alternative-databases/postgresql.html) if you want to take a look and see how this would render if the pull request is accepted. I welcome your input on the way the code examples are laid out, etc.