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

chore: upgrade to markdownlint v0.39.0 #3200

Merged
merged 1 commit into from
Mar 20, 2024

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Mar 20, 2024

Maybe closes #2212

Motivation

#3198 (comment)

@rootulp rootulp self-assigned this Mar 20, 2024
@rootulp rootulp marked this pull request as ready for review March 20, 2024 20:17
Copy link
Contributor

coderabbitai bot commented Mar 20, 2024

Walkthrough

Walkthrough

The overall change focuses on enhancing the Markdown linting setup and documentation formatting within a project. It updates the markdownlint-cli version, modifies the linting configuration for rule MD055, and improves table formatting across various documents, including the README and architectural decision records (ADRs). These adjustments aim to enhance readability, enforce consistency, and ensure the Markdown lint CI effectively identifies and fails on non-respected linting rules.

Changes

Files Change Summary
.github/workflows/markdown-linter.yml Updated markdownlint-cli version from 0.32.1 to 0.39.0.
.markdownlint.yaml Modified MD055 rule to enforce "leading_and_trailing" style.
README.md Updated table formatting and markdownlint version.
docs/architecture/.../adr-013-...-padding.md, docs/architecture/.../adr-015-...-size.md, docs/architecture/.../adr-019-...-schedule.md Adjusted table formatting for clarity and uniformity.

Assessment against linked issues

Objective Addressed Explanation
Markdown lint CI not failing on non-respected rules (#2212)

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Note: Auto-reply has been disabled for this repository by the repository owner. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@rootulp rootulp enabled auto-merge (squash) March 20, 2024 20:18
Comment on lines 119 to 126

Cosmos Hub (a.k.a Gaia) sets `BlocksPerYear` to 4,360,000 [here](https://github.com/cosmos/gaia/blob/8a522e98a2863205cf02fb97f8ad27d091670b9d/docs/governance/current-parameters.json#L86). Cosmos Hub has a block time 6.353 ms. Numia data doesn't have complete data for 2021 so we'll examine 2022:

Year | BlocksPerYear | Actual # of Blocks | Skew
-----|---------------|--------------------|------
2022 | 4,360,000 | 4,580,463 | 5.05%
| Year | BlocksPerYear | Actual # of Blocks | Skew |
|------|---------------|--------------------|-------|
| 2022 | 4,360,000 | 4,580,463 | 5.05% |

Source: [Numia query](https://console.cloud.google.com/bigquery?sq=611612269782:f0c42f9584c448c78a4ec5f118c2091c)
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [100-100]

The verb "trigger" should agree with its subject, suggesting a revision to "triggers". For example: "...when a validator or delegator triggers a withdrawal..."

- At some point in the future when a validator or delegator trigger a withdrawal, tokens are withdrawn from...
+ At some point in the future when a validator or delegator triggers a withdrawal, tokens are withdrawn from...

Comment on lines +52 to +57
| Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |
Copy link
Contributor

Choose a reason for hiding this comment

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

The table formatting changes improve readability and consistency. However, consider adding a header row to enhance clarity further.

+ | Blob size range | Subtree root width | Index multiple | Worst-case padding |
  |---------------------------------|------------------------|-----------------|--------------------|
  | blob <= 64                      | 1                      | multiple of 1   | 0                  |
  | 64 < blob <= 128                | 2                      | multiple of 2   | 1                  |
  | 128 < blob <= 256               | 4                      | multiple of 4   | 3                  |
  | 256 < blob <= 512               | 8                      | multiple of 8   | 7                  |

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
| Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |
| Blob size range | Subtree root width | Index multiple | Worst-case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |

Comment on lines 49 to 60

For example, assume `SubtreeRootThreshold = 64`. This would mean that the blobs smaller than the `64` can start at an index that is a multiple of one and therefore introduce zero padding. Blobs that are larger than `64` but smaller than `64 * 2 = 128` can use an index that is a multiple of 2 to get a maximum of 1 padding share. Blobs that are larger than `64 * 2 = 128` but smaller than `64 * 4 = 256` can use an index that is a multiple of 4 to get a maximum of 3 padding shares and so on.

Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding
--------------------------------|------------------------|-----------------|-------------------
blob <= 64 | 1 | multiple of 1 | 0
64 < blob <= 128 | 2 | multiple of 2 | 1
128 < blob <= 256 | 4 | multiple of 4 | 3
256 < blob <= 512 | 8 | multiple of 8 | 7
| Blob size (in number of shares) | Subtree root width[^2] | Index in square | Worst case padding |
|---------------------------------|------------------------|-----------------|--------------------|
| blob <= 64 | 1 | multiple of 1 | 0 |
| 64 < blob <= 128 | 2 | multiple of 2 | 1 |
| 128 < blob <= 256 | 4 | multiple of 4 | 3 |
| 256 < blob <= 512 | 8 | multiple of 8 | 7 |

Note: the threshold doesn't need to be `64` and the implementation versions this constant so that it is possible to modify over time.

Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-15]

Consider adding a comma before "but" when it connects two independent clauses, unless they are closely connected and short.

- This is a good improvement but we can reduce padding further.
+ This is a good improvement, but we can reduce padding further.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [17-17]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- analyze the worst case padding for different blob sizes
+ analyze the worst-case padding for different blob sizes

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [23-23]

Add a comma after introductory phrases for clarity.

- In the naive approach if the block proposer aligned blobs one after another
+ In the naive approach, if the block proposer aligned blobs one after another

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

Consider adding a comma before "but" when it connects two independent clauses.

- have the lowest ratio of data to padding but also have small blob inclusion proofs.
+ have the lowest ratio of data to padding, but also have small blob inclusion proofs.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [25-25]

Add a comma after introductory phrases for clarity.

- From now on this threshold is called the `SubtreeRootThreshold`
+ From now on, this threshold is called the `SubtreeRootThreshold`

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [35-35]

This sentence should start with an uppercase letter.

- where `blob` is the length of the blob in shares
+ Where `blob` is the length of the blob in shares

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [65-65]

Add a comma after introductory phrases for clarity.

- If light nodes can process this proof size without a problem then we can use this bound.
+ If light nodes can process this proof size without a problem, then we can use this bound.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [79-79]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- In the current worst case a Celestia block is full of blobs of size 1
+ In the current worst-case a Celestia block is full of blobs of size 1

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [81-81]

Use a hyphen in compound adjectives before nouns. Here, "worst-case padding" should be hyphenated.

- ### Worst Case Padding
+ ### Worst-Case Padding

@rootulp rootulp merged commit e3e11e2 into celestiaorg:main Mar 20, 2024
34 checks passed
@rootulp rootulp deleted the rp/bump-markdown-lint branch March 21, 2024 00:31
ninabarbakadze pushed a commit to ninabarbakadze/celestia-app that referenced this pull request Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Markdown lint CI not failing on non-respected rules
3 participants