With the emphasis on removing aliases in Crystal, I propose removing the context method. It is essentially an alias of describe. While I kinda get what it's meant for, the actual helpfulness of it is less so. There is not really any benefit in using context "with an empty array" do
instead of describe "with an empty array" do
since the context is implied from the description. The different keywords only adds confusion to figure out which one would make the most sense.
I disagree. You're describing something. And that description can have many contexts.
I don't think @vladfaust's argument is enough, because one could justify any alias like that
The actual implementation is identical (or in fact, an alias) but it's not just two different names for the same thing. In the spec framework describe
and context
have different semantics.
Whether we want these semantic difference can be questioned, but it shouldn't matter that the implementation happens to be the same.
I prefer to have both, describe
and context
because it helps writing more expressive specs. The amount of confusion is negligible. The concepts should be pretty much self-explanatory for anyone having a little knowledge about writing specs.
I think this is a very valid point. If the case of "an alias is ok if the method allows for expressiveness" becomes a rule, then how are we to know which aliases are ok? I'd say in this PR, for me Time.now
is a lot more expressive than Time.local
. Even @ysbaddaden brings up the point of wishing we'd keep a few aliases because some aliases can be helpful to be more expressive.
I'm not opposed to keeping both describe
and context
because I use both, but I think whatever the "rule of thumb" is for the language, it should be consistent across all parts of the language. This could be done just with some simple internal documentation like # SEMANTIC ALIAS
or something.
Well, IMHO context
and description
is like this:
name --> meaning -+
\
+-> implementation
/
name --> meaning -+
While Time.local
and Time.now
is like this:
name -+
\
+-> meaning --> implementation
/
name -+
I'm not opposed to keeping Time.now
indefinitely as well. But it's a hard bargain. It's an alias after all and there is no benefit for being able to call two different methods that mean the same thing. It's not like you would use Time.now
in specific situations and Time.local
in others (probably... maybe someone might actually do that 🤷♂️ but that's not what it's intended for).
@straight-shoota people are used to Time.now
, its understandable. Letting the deprecated warning long enough to teach everybody to use Time.local
, then we'll be able to remove the alias :+1:
Me personally, I do use them in different situations, but Time stuff aside, my point is more how can we define when an alias is ok, and when is it not? Like in this case, context and description are in fact an alias, but we say it's ok because of their semantic meaning being different. If that's the case, we (as a community) should have some way to visibly see that. So if we had some sort of comment tag like :nodoc:
:semantic:
or whatever that said "this alias is ok" vs someone submitting a PR to remove context because they saw a chance for a cheap PR to remove an alias since the rule is "no aliases".
I don't think we need a way to declare this. It should be evident from the documentation. We're just lacking that for description
and context
😆
I trust in the competence of maintainers and experienced contributers to point out the error of such cheap PRs.
The idea of removing some aliases from the stdlib was:
map
, some prefer collect
and that's a style clash)array.first(10)
and array.take(10)
you'll have to implement those aliases for containers that behave like Array
and that's a bit annoying)For spec, describe
and context
are probably fine. It's not like you prefer one or another, they convey a different meaning: describe
to describe an object or some feature, context
to test some things in a given context. It's different to an alias like collect
for map
because then it becomes a user preference, but context
and describe
are not a preference.
Seems consensus is to keep it. I'll go ahead and close this.
Most helpful comment
The actual implementation is identical (or in fact, an alias) but it's not just two different names for the same thing. In the spec framework
describe
andcontext
have different semantics.Whether we want these semantic difference can be questioned, but it shouldn't matter that the implementation happens to be the same.
I prefer to have both,
describe
andcontext
because it helps writing more expressive specs. The amount of confusion is negligible. The concepts should be pretty much self-explanatory for anyone having a little knowledge about writing specs.