General documentation

[0xdcota]

Preparing for code4rena:

• [x] IRouter and routers derivatives -> @brozorec
• [x] IChief -> @brozorec
• [x] IVault and derivatives, factories, vaultpermissions -> @DaigaroCota
• [x] ISwapper -> @DaigaroCota
• [x] ILendingProvider and derivatives, including related provider interfaces and libraries -> @pedrovalido
• [x] IFlasher and derivatives -> @pedrovalido
• [x] AddrMapper -> @pedrovalido

[brozorec]

@DaigaroCota @pedrovalido I started reviewing the comments in Chief. Plz check carefully the changes and let me know if this format works for you and what we can improve.

Throughout the whole code base, we use different ways to comment, different punctuation, there are missing natspecs for some functions, etc.

After we validate the format, we MUST stick to the same one. If we fail to do so, it's only a sign of negligence and bad coding practices.

[0xdcota]

I reviewed and it looks ok on my side.

The questions I have now is:

• Should we all commit in the same branch ? protocol/docs/review-code-comments
• Should we comment in a similar fashion the internal functions? In this case I recommend we focus the comments as @dev instead of @notice. From what I recall @notice is intended for public\external functions.

[pedrovalido]

Should we all commit in the same branch ? protocol/docs/review-code-comments? I say yes in the same branch as we all have different files. So, it shouldnt be a problem to do so

Should we comment in a similar fashion the internal functions? In this case I recommend we focus the comments as @dev instead of @notice. From what I recall @notice is intended for public\external functions. Yes, let's do a standard for us: @notice for external functions and @dev for internal function. There are some use cases where external functions have internal comments. We can keep both comments (@notice and @dev) at external functions as long as the comment is intended to be used only in internal implementation. Lmk what you think

[brozorec]

• I suggest we work on different branches and PRs to easier review each other.

• I agree on the usage of @dev and @notice, I wasn't aware of the difference in their usage.

[brozorec]

@param assets Amount of assets to deposit.

or

@params assets amount of assets to deposit.

Which one should we use? @DaigaroCota @pedrovalido

[pedrovalido]

I would say the bottom one: @params assets amount of assets to deposit.

[pedrovalido]

The naming branch convention for us to work on different branches can be protocol/docs/review-code-comments-name-of-the-task (following Daiagaro's branch). The name of the task can be the name above on the list of tasks (shortened in same cases). What do you think?

[pedrovalido]

Just noticed some of the comments have "." at the end. Are we adopting this for all comments?

[0xdcota]

Just noticed some of the comments have "." at the end. Are we adopting this for all comments?

I suggest we do.

This is an example of a proper sentence.

[pedrovalido]

Also noticed some types are being used like this in the comments - { IExample } Should we adopt this as well?

[pedrovalido]

Note to self:

• [x] write AddrMapper need to be properly configured where it is fit
• [x] custom errors
• [x] finish sentences with "."

[brozorec]

I would say the bottom one: @params assets amount of assets to deposit.

Sorry to chime in later here but I think it doesn't feel natural to read at all. Adding a "." at the end of a phrase means it's a sentence which means it has to start with a capital letter.

I'm aware @DaigaroCota and @pedrovalido, you have already formatted many files so I suggest you don't re-format it again, but we apply it only for the new ones.

Here's my proposal on how to write it: @param assets Amount of assets to deposit.

[pedrovalido]

Actually I don't think @param assets Amount of asset to depositwould qualify to be a sentence. I suggest we do @param assets amount of assets to deposit (removing the "." and the capital letter on the amount). This is what makes the most sense to me. Let me know what you think @brozorec @DaigaroCota

[pedrovalido]

I also suggest we remove "." from the titles as they're not sentences. What do you think?

Example @title Hundred Lending Provider. becomes @title Hundred Lending Provider

[0xdcota]

On my side ok, to remove "." from titles and stick to this format:

@title Hundred Lending Provider

Also, ok to remove capitalized letters from @param statements, and period at the end. However, trying to use the param name itself as part of the statement.

@param assets amount to deposit

For @dev, or @notice we keep formal sentences, including "Requirements" statements. Capitalized first word + ended in ".".

@notice Returns the amount of shares user has.

Requirements:
- Must check `provider` is not address zero.