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

[Feat] - Add Contributors Page, Updated Docusaurus, Fix Linters #175

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

adithyaakrishna
Copy link
Contributor

Description:

Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Signed-off-by: Adithya Krishna <[email protected]>
Copy link
Collaborator

alabulei1 commented Oct 16, 2023

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Overall, the patch includes several different sets of changes.

The first set of changes focuses on adding a new Markdown linting workflow and associated npm scripts. The potential issues to address include ensuring the appropriateness of the .markdown-lint.yml configuration file, verifying the security and reliability of third-party actions used, checking the dependencies and functionality of the added npm scripts, reviewing the flow of changes to avoid conflicts or unintended consequences, and considering the need for tests. Additionally, the potential performance impact of pre-commit hooks should be evaluated, and proper documentation should be provided.

The second set of changes involves documentation updates. The potential problems to address include inconsistent use of references to issues, lack of clarity and formatting for notes, and the possibility of introducing errors or inconsistencies.

The third set of changes relates to the addition of new files and renaming existing ones in the documentation structure. While there don't seem to be any potential problems, it is important to review the contents of the new and modified files, ensure proper linking and integration, maintain consistent naming conventions and directory structure, and consider any dependencies or interrelationships.

Lastly, the patch includes straightforward updates to Docusaurus and its dependencies, as well as the Algolia packages, with no potential problems identified.

In summary, the patch encompasses various changes, including the addition of a Markdown linting workflow, documentation updates, modifications to the documentation structure, and dependency updates. The potential issues and errors are primarily focused on ensuring appropriateness, security, reliability, functionality, performance impact, and proper documentation.

Details

Commit e3b5c2bb1d09c0ce2ca47d919932e757e8cb78e8

Key changes in the patch:

  1. Added a new workflow called "Markdown Lint" in the .github/workflows/lint.yml file. This workflow is triggered when there are changes in Markdown files (**/*.md). It uses the DavidAnson/markdownlint-cli2-action action to perform Markdown linting using a configuration file located at .github/linters/.markdown-lint.yml.

  2. Modified the .husky/pre-commit file to include a new command npm run lint:md-fix after npm run lint-staged --allow-empty. This command runs Markdown linting and fixes any issues found in the staged Markdown files.

  3. Added new npm scripts in the package.json file:

    • lint:md: Runs Markdown linting using the markdownlint-cli2 package on all Markdown files (**/*.md).
    • lint:md-fix: Runs Markdown linting and fixes any issues using the markdownlint-cli2-fix package on all Markdown files (**/*.md).

Potential problems:

  1. It's unclear what the .github/linters/.markdown-lint.yml configuration file contains. Reviewers should ensure that the configuration is appropriate for the project's needs.

  2. Reviewers should check if the actions used (actions/checkout, tj-actions/changed-files, and DavidAnson/markdownlint-cli2-action) are reputable and have been widely used. It's important to verify the security and reliability of third-party actions.

  3. It's important to review the changes to the package.json file and ensure that the newly added npm scripts (lint:md and lint:md-fix) have appropriate dependencies installed and the scripts are functioning correctly.

  4. Reviewers should examine the flow of the changes and verify there are no conflicts or unintended consequences with the existing linting workflows and scripts.

  5. The patch does not include any tests for the new markdown linting functionality. Reviewers should consider if tests are necessary and request the developer to include them if needed.

  6. The patch modifies the .husky/pre-commit file, which suggests that it introduces automatic linting and fixing on pre-commit hooks. This could potentially slow down the commit process if linting and fixing take too long. Reviewers should ensure that the performance impact is acceptable.

  7. It is crucial to analyze the documentation and guidelines to understand how the Markdown linting process should be integrated into the development workflow correctly. Documentation in the form of README updates or pull request descriptions should be present to guide developers on how to use the new linting feature.

These potential problems should be addressed before merging the changes.

Commit 617a9d982aee3a8a62d02e920cd973514a7cd96d

Key changes:

  1. Updated documentation in docs/contribute/contribute.md to include references to issues and check the CI.
  2. Added information about feature requests process in docs/contribute/contribute.md.
  3. Added a note and fixed formatting in docs/embed/go/reference/0.9.x.md.
  4. Added a note and fixed formatting in docs/start/usage/serverless/vercel.md.
  5. Added a note and fixed formatting in i18n/zh/docusaurus-plugin-content-docs/current/start/usage/serverless/vercel.md.

Potential problems:

  1. Inconsistent use of references to issues in the documentation.
  2. Lack of clarity in the use of notes and lack of consistent formatting for notes.
  3. The changes in the documentation may introduce inconsistencies or errors if not reviewed thoroughly.

Overall, the changes seem fine, but it is recommended to review the consistency in referencing issues and the use of notes.

Commit 5d9a4deb16bcb6613e5a61f31afbde131fd0abcf

Key changes in this patch include:

  1. Added a new file docs/contribute/community/_category_.json to define a label and link for the "WasmEdge Users and Contributors" category.
  2. Added a new file docs/contribute/community/contributors.md to display a page for recognizing and thanking contributors.
  3. Renamed the file docs/contribute/users.md to docs/contribute/community/users.md.
  4. Added a new file i18n/zh/docusaurus-plugin-content-docs/current/contribute/community/_category_.json to define the same label and link for the Chinese translation of the "WasmEdge Users and Contributors" category.
  5. Added a new file i18n/zh/docusaurus-plugin-content-docs/current/contribute/community/contributors.md to display a translated version of the contributors page.
  6. Renamed the file i18n/zh/docusaurus-plugin-content-docs/current/contribute/users.md to i18n/zh/docusaurus-plugin-content-docs/current/contribute/community/users.md.

Potential problems/considerations:

  1. There don't seem to be any potential problems with these changes, as they mainly involve adding new files and renaming existing ones. However, it's important to review the contents of the new and modified files to ensure they meet the project's requirements and standards.
  2. It would be helpful to check if the newly added files are properly linked and integrated with the existing documentation structure.
  3. The file naming conventions and directory structure should be consistent with the project's conventions.
  4. If there are any dependencies or interrelationships between the modified files and other parts of the project, those should be taken into account and reviewed as well.

Overall, it seems like a straightforward addition of a new page and directory structure.

Commit 64bf3cf892f06a180f1cd96b0e58b47516600fd7

Key changes:

  • Update Docusaurus to version 2.4.3
  • Updated dependencies of Docusaurus packages
  • Updated dependencies of Algolia packages

Potential problems:

  • There don't seem to be any potential problems in this patch. The changes are straightforward and involve updating dependencies to newer versions.

@adithyaakrishna adithyaakrishna changed the title [Feat] [Feat] - Add Contributors Page, Updated Docusaurus, Fix Linters Oct 16, 2023
@adithyaakrishna
Copy link
Contributor Author

This PR is a WIP as Linter is Failing

- **Feature Requests**: Your innovative ideas have been invaluable in driving the project forward.
- **Community Support**: Those who have helped by answering questions, providing support, and moderating discussions, you've created a welcoming community for everyone.

| Name | Role | Organization | LinkedIn | Twitter | GitHub |
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you can't list all the maintainers, then it's meaningless.

Copy link
Contributor Author

@adithyaakrishna adithyaakrishna Oct 17, 2023

Choose a reason for hiding this comment

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

@alabulei1 Its a WIP, I dont have the info regarding the Role Organization and the social media links. Could you please share with me the same?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have this information either.

@alabulei1
Copy link
Collaborator

Hi @adithyaakrishna

Could you please take a look at the CI tests? Thanks.

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.

Bug: Linter not enabled Bug: The linter is not the same as the original repo
2 participants