search

serokell / haskell-with-utf8

Get your IO right on the first try
https://serokell.io/blog/haskell-with-utf8
52 stars 3 forks source link

[INT-146] First version imported from nixfmt #1

Closed kirelagin closed 4 years ago

kirelagin commented 4 years ago

We have had all this code in Nixfmt for quite some time now. The plan was to eventually extract it into a separate library, and the time has come!

kirelagin commented 4 years ago

Problem: this library does not exist. Solution: write code to solve the problem that this library is supposed to solve.

@serokellcao I am hoping you were joking, but you were probably not. Well, ok.

The commit messages discipline exists to make sure that it is easy to understand what changes are happening in the project, in particular make it easier for people looking at the history understand the evolution of the project on the large scale, and also help people looking at a PR understand what is happening and why on the small scale.

Asking for the first commit in the repo (or, well, the first commit that has to do with “business logic”) is not smart for two reasons:

  1. It serves absolutely no purpose (apart from blindly following the guidelines without putting a tiny bit of though on the reason the guidelines exist). When you start with having nothing and then suddenly have something there is nothing in this process that needs an explanation, because the change is obvious.
  2. There is just nothing that can be written in the first commit message that would make sense. See the first paragraph of this comment. This is a problem in the guideline that it does not explicitly say that it does not apply to the first commit message and assumes that the person reading it will use their discretion.
serokellcao commented 4 years ago

No, I wasn't joking @kirelagin. I didn't even read anything in your commit. You probably should have explained there that there is README for more information. Oh wait, it has a broken link to a non-existing blog post.

If you ask to review something, it probably means that you're not committing scaffolded code, but instead it has some "business logic", which can't be an initial commit in any repo. Initial commit should be scaffolding, README, LICENSE, etc.

From what I see is that you took some code from somewhere else and started putting it into a separate library. That doesn't constitute an initial commit and should follow the guidelines of explaining what the hell you're doing to the general audience.

Believe me, you will get more people eager to read your +427|-0's if you explain to them what is it about.

kirelagin commented 4 years ago

explaining what the hell you're doing to the general audience

There is no point in explaining what this code does to the general audience. The point of this library is “take this code and use it because it’s good”. The explanation of why using it is a good idea is quite long and will be in a blog post, but the blog post needs to be written first and also needs to contain a link to this library ready to be used (that is, with this PR merged).

I guess, what I’m trying to say is that it was not my intention to ask “the general audience” to review it. I was hoping that those who know what is going on (that is, probably, @Lucus16, @gromakovsky, and @Martoon-00, IIRC, most likely someone else) would review it, and those who don’t would just walk by.

serokellcao commented 4 years ago

Then you probably shouldn't ask for a review in #haskell and use “request review” feature of GitHub highlighting specific people.

Kindest regards, Jonn Mostovoy, Co-founder and Chief Architect at Serokell.

On Sun, Feb 16, 2020 at 9:16 AM Kirill Elagin notifications@github.com wrote:

explaining what the hell you're doing to the general audience

There is no point in explaining what this code does to the general audience. The point of this library is “take this code and use it because it’s good”. The explanation of why using it is a good idea is quite long and will be in a blog post, but the blog post needs to be written first and also needs to contain a link to this library ready to be used (that is, with this PR merged).

I guess, what I’m trying to say is that it was not my intention to ask “the general audience” to review it. I was hoping that those who know what is going on (that is, probably, @Lucus16 https://github.com/Lucus16, @gromakovsky https://github.com/gromakovsky, and @Martoon-00 https://github.com/Martoon-00, IIRC) would review it, and those who don’t would just walk by.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/serokell/haskell-utf8/pull/1?email_source=notifications&email_token=AK2DJQ5JGXXEMWESB7VGRULRDDR5DA5CNFSM4KV5WPZ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEL37TIQ#issuecomment-586676642, or unsubscribe https://github.com/notifications/unsubscribe-auth/AK2DJQ6OLIHRMZEWKSNUKUTRDDR5DANCNFSM4KV5WPZQ .

Lucus16 commented 4 years ago

I have some doubts about the name. Unless you're willing to support expanding the library with a lot of other UTF-8 functionality, I recommend choosing a more specific name and leaving utf8 to someone who's willing to do that. I don't have time to review this anytime soon unfortunately.

kirelagin commented 4 years ago

I couldn’t come up with a better name. Also, the idea of the library is “use UTF-8 everywhere (where it make sense)”, so in this regard, I think, the name works quite well. I’m not really sure what UTF-8 functionality can be useful to add, other than tools that simplify using it.

kirelagin commented 4 years ago

Ok, I am going to merge this now. I have already made a lot of other changes, including major changes to the interface and the internal logic. It’s only the very beginning of the library and making reviews of intermediate versions was probably not a good idea (my excuse is, I did not expect it to change that much of the course of two days).

So I’ll integrate all the changes that I made and, hopefully, will get a round of reviews when it is all more-or-less stable.