search

ChainSafe / engineering-handbook

ChainSafe Engineering Handbook
https://handbook.chainsafe.io
Apache License 2.0
6 stars 2 forks source link

Development/tech-stack/Go: Project structure #16

Closed qdm12 closed 2 years ago

render[bot] commented 2 years ago

Your Render PR Server URL is https://engineering-handbook-pr-16.onrender.com.

Follow its progress at https://dashboard.render.com/static/srv-c961d0j19n08vj87ci30.

mpetrunic commented 2 years ago

Could you add unknown words to dictionary.txt so that spellchecker passes

qdm12 commented 2 years ago

Done @mpetrunic thanks for the tip

kalambet commented 2 years ago

@qdm12 This is great! Thank you so much!

Couple of things that I think might be worth mentioning:

  1. On the code structure. In addition to what you wrote there is Standard Package Layout approach that is de facto starndard for quite some time already: https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1

  2. On the SDK/API contracts design. Nice piece on the Functional Options approach: https://www.sohamkamani.com/golang/options-pattern/

kalambet commented 2 years ago

And on the linting perspective I think it worth mentioning: https://github.com/golangci/golangci-lint

Not sure about specific config but deafult one is pretty strict and fair.

qdm12 commented 2 years ago

On the code structure. In addition to what you wrote there is Standard Package Layout approach that is de facto starndard for quite some time already:

Anything in particular you would want to add or change to the existing changes?

On the SDK/API contracts design. Nice piece on the Functional Options approach:

Let's address this in another PR after that one gets merged. Also I'm not such a fan of these since it obscures the available settings unlike a big dumb settings structure. It's also harder to assert in unit tests than a settings struct. Although I use it sometimes, such as for logger constructors since I use these often.

And on the linting perspective I think it worth mentioning

Let's address this in another PR after that one gets merged.

kalambet commented 2 years ago

Anything in particular you would want to add or change to the existing changes?

I would say, in addition to what was written already we can put this whole link in there.

cmd/internal/pkg/lib considerations are mostly from the language modules scpecifics perspective and Standard Package Layout is more about business logic perspective, http handlers and so on. I don't see any reason to sopy paste the whole article but adding business logic specific layout might be useful.

On the other things, yes, let's address it in different PR.

qdm12 commented 2 years ago

@kalambet I created 3 subsections in the project structure section:

- [Project structure](#Project-structure)
  - [Modules layout perspective](#Modules-layout-perspective)
  - [Business logic perspective](#Business-logic-perspective)
  - [Other tips](#Other-tips)

With the Medium link in the Business logic susbection. On that note, I think it would be better to have short summary/bullet points instead of external links, since in my experience people are lazy to click through links :smile:

Also on another note, would it be fine to have a go directory with different markdown files? That would be easier to navigate and maintain.

mpetrunic commented 2 years ago

Also on another note, would it be fine to have a go directory with different markdown files? That would be easier to navigate and maintain.

In theory yes, and feel free to do it. TBH, I would prefer to have it consistent across all tech stacks but maybe it won't be necessary.

kalambet commented 2 years ago

@qdm12 thank you so much! Added a suggestion with a small intro.

kalambet commented 2 years ago

CI is faliling and I'm on it. Machine does not like "de facto", but machine shall not stand between humans and their words!!! 🤖

qdm12 commented 2 years ago

Awesome thanks everyone.

Also after some thoughts, I think it's better to keep the Go tech stack document as one file. It might end up being some huge file, but I personally like to scroll and CTRL+F in a single file instead of clicking around on different files.

mpetrunic commented 2 years ago

Awesome thanks everyone.

Also after some thoughts, I think it's better to keep the Go tech stack document as one file. It might end up being some huge file, but I personally like to scroll and CTRL+F in a single file instead of clicking around on different files.

FYI there is search field in top right corner :)