sablier-labs / flow

🍃 Smart contracts of the Sablier Flow protocol.
Other
8 stars 0 forks source link

Add newlines between tag types #181

Closed smol-ninja closed 3 months ago

smol-ninja commented 4 months ago

Discussed in https://github.com/sablier-labs/v2-core/discussions/906

Add newlines between tag types to improve readability.

/// @notice Creates a new MerkleLockup campaign with a LockupLinear distribution.
/// @dev Emits a {CreateMerkleLL} event.
/// @param baseParams Struct encapsulating the {SablierV2MerkleLockup} parameters, which are documented in
/// {DataTypes}.
/// @param lockupLinear The address of the {SablierV2LockupLinear} contract.
/// @param streamDurations The durations for each stream.
/// @param aggregateAmount The total amount of ERC-20 assets to be distributed to all recipients.
/// @param recipientCount The total number of recipients who are eligible to claim.
/// @return merkleLL The address of the newly created MerkleLockup contract.

becomes

/// @notice Creates a new MerkleLockup campaign with a LockupLinear distribution.
///
/// @dev Emits a {CreateMerkleLL} event.
///
/// @param baseParams Struct encapsulating the {SablierV2MerkleLockup} parameters, which are documented in
/// {DataTypes}.
/// @param lockupLinear The address of the {SablierV2LockupLinear} contract.
/// @param streamDurations The durations for each stream.
/// @param aggregateAmount The total amount of ERC-20 assets to be distributed to all recipients.
/// @param recipientCount The total number of recipients who are eligible to claim.
///
/// @return merkleLL The address of the newly created MerkleLockup contract.
smol-ninja commented 3 months ago

I changed my mind about this. It does not look good when you have one notice, one dev, one param and one return.

/// @notice Calculates the amount that the sender owes on the stream, i.e. if more assets have been streamed than
/// its balance, denoted in 18 decimals. If there is no debt, it will return zero.
/// @dev Reverts if `streamId` references a null stream.
/// @param streamId The stream ID for the query.
/// @return debt The amount that the sender owes on the stream.
function streamDebtOf(uint256 streamId) external view returns (uint128 debt);

vs

/// @notice Calculates the amount that the sender owes on the stream, i.e. if more assets have been streamed than
/// its balance, denoted in 18 decimals. If there is no debt, it will return zero.
///
/// @dev Reverts if `streamId` references a null stream.
///
/// @param streamId The stream ID for the query.
///
/// @return debt The amount that the sender owes on the stream.
function streamDebtOf(uint256 streamId) external view returns (uint128 debt);

Its quite useful when you have multiple params so that makes it a bit subjective. So I am happy to just refactor wherever are multiple params instead of doing it for all as a thumb rule.

andreivladbrg commented 3 months ago

For me it's the same for all, I don't have a preference, do as you wish.