Approach to documenting our know-how

[radeusgd]

There are some various opinions about how we should approach documenting the knowledge related to our internal libraries, and there was a proposal to start a discussion, so I'm starting one.

---

I'd like to discuss how we want to approach 'storing' and searching for information related to our internal libraries - mostly internals of the engine/interpreter, but it can probably also apply to deeper internals of the libs (internal Enso helpers, the polyglot Java libs like Table etc.).

---

• Java style guide on comments

[radeusgd]

@JaroslavTulach is suggesting the view that Pull Requests and Issues are part of the project documentation.

The explanation is important. However I believe consistent explanation of motivations in the PR is even better than random comments spread thru the source code. Selective Cluelessness asks for making a "knowledge increasable". Should the code not be self explanatory, then all one needs to do to use git blame and locate the #6255 PR.

(...) Duplicating randomly identified parts of such description in the source code makes little sense. It'd be even more misleading than useful.

Let's treat Pull Requests and Issues as natural part of the project. Let's learn how to locate them when seeing incomprehensible source code.

[Akirathan]

Let me contribute to the discussion. I am going to structure my message into two sections; the first one describes what I believe is the best possible approach towards documentation and code, and the second one replies to the comments above.

The reason why the motivation section is so long is that I disagree with the approach that we should refactor code only if it increases performance or if it is part of the public API.

My goal and my motivation

I think that we can all agree that issues, pull request, discussion, etc. are all relevant part of the project. However, what I am advocating for (and I believe @radeusgd is also advocating for) is a Clean Code.

There is a great and inspiring book by Robert C. Martin about what he calls clean code. In the very first introductory chapters, there are many quotations from very well known and respected authorities in the software engineering field, like Bjarne Stroustrup, Grady Booch, Dave Thomas, and others. And to all my understanding, what they all agree on is that the cost of a bad code is huge. The following two paragraphs, along with the figure, are quotations from that book, which I think stress out my point:

---

The total cost of owning a mess

If you have been a programmer for more than two or three years, you have probably been significantly slowed down by messy code. The degree of the slowdown can be significant. Over the span of a year or two, teams that were moving very fast at the beginning of a project can find themselves moving at a snail’s pace. Every change they make to the code breaks two or three other parts of the code. No change is trivial. Every addition or modification to the system requires that the tangles, twists, and knots be “understood” so that more tangles, twists, and knots can be added. Over time the mess becomes so big and so deep and so tall, they can not clean it up. There is no way at all.

As the mess builds, the productivity of the team continues to decrease, asymptotically approaching zero. As productivity decreases, management does the only thing they can; they add more staff to the project in hopes of increasing productivity. But that new staff is
not versed in the design of the system. They don’t know the difference between a change that matches the design intent and a change that thwarts the design intent. Furthermore, they, and everyone else on the team, are under horrific pressure to increase productivity. So they all make more and more messes, driving the productivity ever further toward zero:

Why clean code matters?

The ratio of time spent reading vs. writing code is well over 10:1. We are constantly reading old code as part of the effort to write new code. Because this ratio is so high, we want the reading of code to be easy, even if it makes the writing harder. Of course, there’s no way to write code without reading it, so making it *easy to read actually makes it easier to write.

---

So I claim that taking a little extra effort to comply with some best practices in the field, which often require a bit of refactoring like identifier renaming, splitting code into multiple methods, etc., will pay off heavily in the future, as it means that time to read, and understand the code, will decrease.

Since we all strive to make Enso a good, usable, and stable product, we should spend some effort in order to make the code base as readable and understandable as possible, so that the maintenance cost in the future is minimized.

Documentation in VCS?

Now, back to @JaroslavTulach 's comment about the information in VCS. I agree that from time to time, we have to look into some older PRs in order to properly understand the motivation of some code (one particular use case that I can think of now is a removal of some old unused code). But I don't see a reason why should I do that in order to understand a piece of code that could have been refactored, and documented, right away. Even in our Java code style guide, we have a Source Notes section that says that if a piece of code might be too difficult to understand for any reasons, a documentation should be written in the source code.

[radeusgd]

I agree with the above.

To sum up and partially reply - I think that yeah in some cases we do need to 'become an archealogist' - if we want to understand some more complex motivation of a thing. I guess that is fine.

But when I look at a piece of code - I shouldn't need to perform archaeology to understand how it works. Adding comments or renaming methods to be more readable is not duplicating random parts of the design.

I think our documentation is lacking and this will bite us but at the same time I'm not proposing having to write lengthy design documents for every fix - I'm not sure what is the best solution here. Maybe we should try to have some doc summing up the overall design directions and try to update it from time to time? I think it may prove to be useful at some point, but I'm not pushing for it.

I feel like maybe we misunderstood on the PR too?

Because the #6255 (comment) is in my PR. It needs to be somewhere as a whole,

True! But what I think me and @Akirathan meant is not necessarily to describe in detail why we are sometimes adding frames and why we introduced the inlining. The request was to document the fact that 'having a VirtualFrame argument, influences if the builtin will be inlined or not'. This is a functionality that is there, but it to understand it one has to dig through the code quite a bit, while a short note on needsFrame could help it.

Also, to understand the motivations for inlining - a link to the PR in a doc comment could also greatly help in the search.

Is there anything problematic which adding links to other pieces of our documentation? Then we know there are resources to look into.

I imagine when introducing a change, I'm not capable to dig through all relevant PRs for every little change - this would take too much time. Especially, as in most cases I will not get much useful information this way anyway. So having links/notes in the relevant places helps figure out when it is worth to do the search.

[JaroslavTulach]

Clean code

Clean code Is indeed desirable. Remaining issue: What methodologies lead us to the clean code?

My favorite one is TDD and particularly its "Write the simplest code that passes the new test". Hence my objection against

• Add forceInline parameter to @Builtin.Method annotation #6293

I feel there is no justification to implement the more complex solution right now when the VirtualFrame check is good enough. Hopefully the documentation problem with VirtualFrame is now addressed by improvements to Javadoc.

[JaroslavTulach]

Few self references from my past:

slowed down by messy code ... the mess becomes so big

That frequently happens. I call it the entropy of software, page 60 of the Practical API Design. One of the best tools to prevent it is to read modularity manifesto. I wish the Enso code was more modular and enforced non-cyclic references more.

difference between a change that matches the design intent

My take on quality. Also available on page 49 of the Practical API Design.

constantly reading old code as part of the effort to write new code

Alas that's what happens to every successful project that lasts years. That's why I'd like us to get ready for becoming archeologist now and always leave tons of "decision evidence" in VCS and issue tracker.

[radeusgd]

The Javadoc addition looks perfect - that's the kind of comment that I thought would help us, thanks!

I guess you are right that sometimes we just have to be archeologists - and I have nothing against leaving decision evidence in the VCS and issues - I think it's a brilliant idea.

I just thought that we should not need to dig very deep to understand not decisions but current implementation. Of course a balance is needed here. Having docs like this added Javadoc is IMO the right balance between too much and too little documentation.

[radeusgd]

In my opinion, the primary place for documentation of code is code itself.

Ideally, every non-obvious behaviour of a method (one that is not apparent from the method name and type), should get a note in the method docstring. And every class whose purpose is not obvious should have a note explaining it.

If there is a decision that is worth explaining, ideally I think it is good to add a short note about it in the code. IMO it makes it easiest to read the code - because when I look at it, I immediately see any related explanations. They should not be pages long - just short notes explaining the intent or reasons to do something. They should not state the obvious, or stuff that is common knowledge in the team - they should be used for places where it was not clear. If there was a discussion on a PR how to do something - IMO such a discussion suggests that something was not clear and a short note about it is a good idea.

The code should be readable and express the intent well and thus need little comments. But sometimes it is not possible due to performance restrictions, or just complex logic - in that case some explanation what is happening helps immensely to understand what was the intention of the person writing it. Some examples from our libraries: (1), (2), (3), (4), (5). And for example from @hubertp's code: (6), (7).

For more complex concepts, spanning across many files or projects, we have the docs directory where we can explain the overall architecture. This directory is still updated, but I feel like it is getting much less traffic than some time ago. For example, Function call flow beautifully explains how our function calling works - I'm worried how up-to-date that is, but back in the day it helped me understand this architecture - which was a bit complex. Then we have this awesome docs of the annotations for builtins.

I definitely don't want to flood us with documentation, but I think we are lacking it in some places, making onboarding of new developers potentially harder and worsening our 'bus factor'.

[JaroslavTulach]

place for documentation of code is code itself.

Some methodologies advocate self-documenting code and some (extreme) methodologies even prevent comments in the code claiming that then the identifier names, function names & etc. get better as a compensation for missing // comments.

In any case Java also offers Javadoc. That is a standard and suitable place for (even internal) API documentation. Hence I put the explanation of the VirtualFrame argument behavior into Javadoc of @BuiltinMethod annotation. IDEs shall offer you the text when using/hovering over the @BuiltinMethod usage.

Having the documentation visible at usage site is clearly a practical advantage of having good Javadoc. Writing Javadoc is something I can only encourage in spite of belonging more to the self-documenting code camp.