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

Add namespace conventions #2

Merged
merged 3 commits into from Apr 25, 2017
Merged

Add namespace conventions #2

merged 3 commits into from Apr 25, 2017

Conversation

sovelten
Copy link

No description provided.

README.md Outdated
<sup>[[link](#commonly-used-namespaces)]</sup>

* <a name="common-namespace-segments"></a>
Prefer these abbreviations for common namespace segments used inside services:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's squad-wise. a discussion happened about this and I think we didn't get to a company-wide conclusion

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of this PR is to try to reach a company-wide conclusion. (even if the conclusion is that each squad should decide it)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against the l.foo/m.bar proposal.

In the services where we've tried to follow the Sierra convention, when there was no ambiguity we used just the last part of the namespace like:

[scatterbrain.controller :as controller]
[scatterbrain.logic.discovery :as discovery]
[scatterbrain.logic.aggregation :as aggregation]

When there is ambiguity, we include previous segments to disambiguate:

[metapod.db.datomic.transaction :as datomic.transaction]
[metapod.logic.transaction :as logic.transaction]

I think this is simpler and easier to remember than taking a prefix of a subsegment of the path as proposed above, and IIRC closer to Sierra's description.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic, Models and Controllers are pretty well understood units within all services, so we thought shortcutting it could improve the readability of the code.

We could submit this point to a vote. For the moment I can withdraw it for the sake of pushing the other points forward.

Copy link
Author

@sovelten sovelten Apr 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another issue: namespaces are very often repeated within logic, controllers, models and db.datomic. Saying some-name instead of l.some-name or logic.some-name might leave the reader wondering from each namespace some-name came from.

Therefore, I think we should make a rule to always use l.some-name or always logic.some-name and never some-name when referring to these namespaces.

README.md Outdated
* <a name="stuart-sierra-namespace-guideline"></a>
Adopt [Stuart Sierra's namespace guidelines](https://stuartsierra.com/2015/05/10/clojure-namespace-aliases)
unless there is a strong reason not to do so or this style guide says otherwise.
<sup>[[link](#stuart-sierra-namespace-guideline)]</sup>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 for Stuart Sierra's namespace guidelines.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it clear the ultimate convention consistency boundary is a service, so avoid mixing aliasing policies in the same service at all costs.

<sup>[[link](#4-positional-fn-params-limit)]</sup>

* <a name="forward-references"></a>
Avoid forward references. They are occasionally necessary, but such occasions

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💅 extra whitespace

Copy link

@rafaeldff rafaeldff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against shortening subsegments ("logic.foo" vs "l.foo").

Other than that, 👍 and 🎉 for the initiative.

README.md Outdated
* <a name="stuart-sierra-namespace-guideline"></a>
Adopt [Stuart Sierra's namespace guidelines](https://stuartsierra.com/2015/05/10/clojure-namespace-aliases)
unless there is a strong reason not to do so or this style guide says otherwise.
<sup>[[link](#stuart-sierra-namespace-guideline)]</sup>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make it clear the ultimate convention consistency boundary is a service, so avoid mixing aliasing policies in the same service at all costs.

README.md Outdated
<sup>[[link](#stuart-sierra-namespace-guideline)]</sup>

* <a name="commonly-used-namespaces"></a>
Keep commonly used namespace abbreviations to maintain consistency between services, such as:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This such as is opening a huge door for inconsistent styles to creep in. If we want a whitelist of exceptions to the aliasing policy, it should be an extensive list. I personally would prefer to avoid it, and only "whitelist" common namespaces for public libraries such as [schema.core :as s] and [datomic.api :as db].

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly about listing some of our namespaces here or not. I do feel strongly this should be an exhaustive list, not a such as.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sierra's namespace guideline itself allows for some deviation (for well estabilished namespaces in the community or very commonly used namespaces). An extensive whitelist might be impossible to maintain, but maybe there is no harm in trying. The ones I listed here were the ones that came up during the discussion with the Bills team, but others could be added as well in a per need basis.

README.md Outdated
<sup>[[link](#commonly-used-namespaces)]</sup>

* <a name="common-namespace-segments"></a>
Prefer these abbreviations for common namespace segments used inside services:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm against the l.foo/m.bar proposal.

In the services where we've tried to follow the Sierra convention, when there was no ambiguity we used just the last part of the namespace like:

[scatterbrain.controller :as controller]
[scatterbrain.logic.discovery :as discovery]
[scatterbrain.logic.aggregation :as aggregation]

When there is ambiguity, we include previous segments to disambiguate:

[metapod.db.datomic.transaction :as datomic.transaction]
[metapod.logic.transaction :as logic.transaction]

I think this is simpler and easier to remember than taking a prefix of a subsegment of the path as proposed above, and IIRC closer to Sierra's description.

@sovelten sovelten merged commit c75f4dd into master Apr 25, 2017
@sovelten sovelten deleted the namespace-conventions branch April 25, 2017 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants