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] - Updated Docusaurus to v2.4.1, Added New Workflows and Migrated to TS #74

Merged
merged 32 commits into from
Jun 8, 2023

Conversation

adithyaakrishna
Copy link
Contributor

@adithyaakrishna adithyaakrishna commented May 22, 2023

Description:

Adithya Krishna added 5 commits May 22, 2023 10:42
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Copy link
Collaborator

alabulei1 commented May 22, 2023

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


The pull request contains various patches that update packages, add new workflows, and modify documentation files. However, some patches raise potential issues that should be addressed to ensure the project's stability. For example, some patches may introduce dependency version mismatches and compatibility issues, or include changes without sufficient testing or documentation.

The most significant finding is the potential problem introduced in the patch that modifies the "build-test.yml" file. Specifically, a run command has been replaced with a script command that concatenates two lines. This could be problematic and lead to errors during script execution.

In conclusion, the pull request seems reasonable and contains some valuable improvements for the project. However, reviewers should pay attention to the patches' potential problems and address them before merging the code.

Details

Commit 68630a1ef7ededaae000b3005a1aaecefd612fd7

The patch updates the Docusaurus version to 2.4.1 and updates some of the dependencies.

Potential problems may include dependency version mismatches, which may cause incompatibility issues, and conflicts with other dependencies. The patch does not include any details about testing, so it is unclear if the changes were tested and verified to be stable and error-free. This may lead to potential issues during implementation or deployment.

Commit 63ecd563a004d820e1db6eb9891525531db374a3

This patch updates the "deploy.yml" file in the ".github/workflows" directory. It includes a new setting for job concurrency and also updates the Node.js version for installation and caching of NPM dependencies.

The "concurrency" setting has been added to ensure that only one deployment is running for a branch at a time. This can avoid any potential conflicts or errors due to simultaneous deployments.

The Node.js version has been updated to "16.18.0" for improved compatibility and security. NPM dependencies are also being cached for faster builds and reduced network traffic.

There are no potential problems identified in this patch. However, if any other workflows are being used, they may need to be updated to ensure compatibility with the new version of Node.js.

Commit b929a8985ac0016b50bf500a2a604b07de7e02fa

This is a GitHub patch that adds a new workflow for build testing. The workflow is triggered on workflow_dispatch, push, and pull_request events, but ignores specific files and directories. The workflow uses the actions/checkout and actions/setup-node libraries to install Node.js and cache dependencies. Finally, it builds the project with npm run build.

There are no major problems with this patch, as it is adding a new workflow that should enhance the testing and building of the project. However, it would be helpful if the patch author included more details about the purpose and scope of the new workflow.

Commit 462695fed4a8ccb8e93c6aaf7f2f852dbb804147

This patch adds a new workflow named "Dependency Review" which runs on every pull request and checks for dependencies. It uses a third-party action named "dependency-review-action".

The key change is adding a new workflow to check for dependencies which is an important step in software development. However, the potential problem might be that the new third-party action "dependency-review-action" is not reviewed or tested. It's advisable to check the source of the action and make sure that it's secure and reliable. Also, the patch may lead to delayed PR merges due to the additional review process.

Commit 75a0e5c88d7317c4cadc1c4c92059c69f50d2958

Key Changes:

  • A new lint workflow has been added to the project with 1 job defined in it.
  • The lint workflow is triggered on pull requests on the main and docusaurus-v** branches.
  • A cache has been added to check, save and check again the NPM dependencies for faster builds.
  • The Node.js version used is updated to 16.18.0.
  • A Check immutable package-lock step has been added to check if the lockfile has changed between cache saves and ensures that it's the same.
  • A problem matcher is added to the workflow.

Potential problems:

  • The lint workflow should also be triggered on pushes to both the main and docusaurus-v** branches for CI/CD operations.
  • It is unclear what causes the cache name to be updated since it's not explicitly defined. This might lead to cache invalidations when not intended.
  • It is unclear what npm commands or configurations are used in the lint:ci and format:diff scripts, and whether these scripts might clash with each other or other workflows.
  • The problem matcher file .github/workflows/cspell-problem-matcher.json is not shared and might not be accessible from other workflows.

Commit 7704a40d54850aae87581fdd9d6d1c4d65b35483

This pull request only contains one patch that renames the 'publish' job to 'test-build' in the '.github/workflows/build-test.yml' file. This is a minor change that shouldn't affect the functionality of the code. However, the commit message does not provide significant information on what motivated the change. Additionally, other key changes mentioned in the Pull Request description were not included in this patch. Therefore, this patch alone cannot be fully reviewed without considering the other patches.

Commit e564ffc043d9e6c157019bba2dad132c22c6c9d4

The patch updates the linting workflow by making some minor changes. Specifically, it removes the check for the immutable package-lock file and adds a code matcher for cspell. The patch also simplifies the command to run the lint and prettier checks.

There are no potential problems identified in the patch, as it is a minor update to the existing codebase. However, it would be worth noting that removing checks for package-lock could potentially lead to issues with dependency management in the future, and therefore should be done with caution.

Commit f89425f26b0b1b0281637a053208bb22105766db

This patch updates the GitHub Actions workflow used to build and test the project. Specifically, it renames the "test-build" job to "Build". It is a minor change that shouldn't raise any issues.

Commit 4324df28b6daed005fb01cd7b1a83ce6ad55d3c3

The patch includes changes to the repository's files that cover various areas such as GitHub issue, workflows, linting, and other documentation files such as the CODE_OF_CONDUCT.md, CONTRIBUTING.md, and README.md files. These changes were made to upgrade Docusaurus to v2.4.1, add new workflows, and migrate to TypeScript.

There are no potential problems identified in the patch.

Commit 9d767475ee68ade28c756b6e48908a32b4cebf1a

The patch updates the formatting of various files in the project, including Docker images for building WasmEdge, Android build guides, and deployment guides. The changes involve mostly reformatting the tables for better readability. There do not seem to be any potential problems.

Commit 4db8661af95ccb03f11df0998f3aedfed14ab798

The patch updates formatted files in the repository, with a total of 389 insertions and 479 deletions. The changes seem to be focused on documentation related to deploying and developing applications with various technologies. The most notable change is the addition of support for the WasmEdge runtime in containerd-shim runwasi. One potential problem is that the patch does not include any tests or documentation changes for the new features, which should be added to ensure maintainability and ease of use for other contributors.

Commit b28a5b488e36855dea04629a03168dd8d19ec18b

This patch primarily involves formatting changes to multiple files in the repository and updating some of the instructions and examples in the documentation for Rust libraries related to AI inference, database, and HTTP service. There don't seem to be any potential problems introduced by these changes. Overall, this appears to be a routine update with no major issues that require attention from the reviewer.

Commit 656a8437ca4f9d302c0d4511fb4ea4bc889331cc

The patch updates the format of several files and fixes a display issue where extra new-line characters had to be added manually at the end of some files for proper formatting. There seem to be no potential problems with these changes.

However, it's unclear what motivated these formatting changes. Without a proper explanation, it can be difficult for other reviewers to understand why these changes were made and whether they were necessary. Additionally, the commit message is not very descriptive and doesn't explain the purpose of the patch.

Commit f0ed81165563b9372b5ea3faf17ae1edb7ad75b6

The patch format seems to be an update to the existing file formatting across the repository for Prettier, a code formatter. There are changes to multiple file types, including workflows, documentation, and config files. The update is done by Adithya Krishna, who has signed off on the changes.

It is unlikely that there will be any potential problems with this patch as it is only related to formatting and not code functionality. However, reviewers may want to check the changes to ensure that they are only related to formatting and do not break anything. They may also want to verify if the formatting adheres to the repository's guidelines.

Commit e45090cc1439afd16b434e0b867dcd33c00ace62

Key changes:

  • The pull request was meant to update Docusaurus to v2.4.1, add new workflows, and migrate to TypeScript. However, this patch shows that the changes were reverted back to v2.4.0.

Potential problems:

  • It is unclear why the changes were reverted, and whether this was done intentionally or accidentally. It is possible that there were issues with the upgraded version that caused the change to be rolled back. Therefore, it is recommended to communicate with the author of the pull request to clarify the reasoning behind this patch.

Commit 482a6b59240460730fc9d05575ed4e70ea59d5a9

The patch updates the Docusaurus version to 2.4.1, adds new workflows, and migrates the codebase to TypeScript. The changes include modifications to the package-lock.json file, build-test.yml configuration file, and package.json.

The most important finding is that the patch introduces a potential problem. Specifically, the changes made to the build-test.yml file replace a run command with a script command which might cause incorrect script execution. The old command "npm run build" has been replaced with a script command that contains two lines. The problem is that the second line should be executed as a separate command, but it is added under the first line with whitespace as separator. This could result in an error.

Another finding is that the update modifies the package-lock.json file by updating the Docusaurus version and various package dependencies. This could cause compatibility issues with some of the dependent packages if they don't support the latest versions.

Overall, the changes seem to be reasonable, but the potential problem with the build-test.yml file should be addressed before merging the pull request.

Commit 33a502681b5bcdc550c80ea70f68153b4f6aec31

The patch updates the Node.js version used in the GitHub workflows from 16.18.0 (specified as a string) to 16.18.0 (specified as a number). The patch also simplifies the build step by removing unnecessary dashes before the npm commands. Additionally, the patch updates the cache check in the build step, replacing "npm install" with "npm i". No potential problems are identified in this patch.

Commit b239c176fe732946d7a599054a43e72235077eee

The patch updates the linting in the build-test workflow file, ensuring the code adheres to certain code style guidelines. The most important finding is that this change can potentially improve the quality and consistency of the code. However, there are no indications of any potential issues or concerns with this patch.

Commit 670fe45ab80d83887037e4a8070d3c63c1828e81

The patch removes the paths constraint from the CodeQL workflow. This constraint would look for changes only within the src and docs directories when running the CodeQL analysis. Removing this constraint means the analysis will now search for changes in the entire repository.

A potential problem could be that this change would result in the CodeQL analysis taking longer to run as it will now search through the entire repository instead of just specific directories. This could impact the speed of the workflow. However, if there are necessary changes outside of those directories, this change is necessary to ensure everything is analyzed.

Commit 44802d22306adcdaf55d67d288cad0f79a241aee

The patch contains changes to several files in the project, including updates to the package-lock.json file, linting updates, and fixes to the src/theme/ReleaseNotes directory.

The most significant change in this patch is the package-lock.json file's update, where over 31,000 lines were removed, and over 31,000 lines were inserted. This could potentially cause issues with the project's dependencies if not tested thoroughly.

The linting updates appear to be minor, with the addition of the react/destructuring-assignment rule set to 'warn.' This change should have no adverse effects on the project.

The fixes made to the src/theme/ReleaseNotes directory again appear to be minor, with only a few lines changed in a file and its associated CSS. This change should also have no adverse effects on the project.

Overall, there may be potential issues with the significant changes made to the package-lock.json file, but these can be addressed through proper testing.

Commit ac2ee9ec08b8cfdb92a35a4cf32fdd018a123b7c

The patch updates the lock files in the project and includes new versions of some dependencies, including "@node-rs/jieba-android-arm-eabi" and "@node-rs/jieba-android-arm64". It also adds new entries for various operating systems and CPUs. There don't seem to be any potential problems in this patch.

Commit ed4b5f326b947e6e1031822cb983be2c839fd42e

This Github patch updates the CodeQL analysis file by making a few changes in its YAML syntax. Specifically, the "on" section has been modified to remove the workflow_dispatch trigger, and the indentation and spacing have been corrected throughout the file. Additionally, there is now a language definition for JavaScript and a matrix with steps to be performed in the analysis job.

The most significant findings are related to the potential impact of the changes on the workflows or features that use or require the CodeQL analysis. Removing the "workflow_dispatch" trigger may affect the manual triggering of the CodeQL analysis workflow; as such, this should be thoroughly tested before merging this pull request. Another issue that requires attention is the change in indentation and spacing. If the syntax is incorrect, the workflow will fail, and so it's essential to make sure that this issue is fixed. Finally, the newly added language and matrix should also be adequately tested to ensure that the analysis is correctly performed, and no errors are introduced into other areas of the codebase.

Commit 24d2e4f45e980119c43c172531c643731d88c71a

The patch fixes linting issues in the project. There are no potential problems identified.

Commit cb8b7b8b4674a94d0c1e7060bbaec8e7ce4479af

The key change in this patch is that the codebase has been migrated to TypeScript. This includes changes to package dependencies and updates to several files such as .eslintrc.js, tsconfig.json, and various JavaScript files that have been renamed and converted to TypeScript. There are no apparent potential problems with the migration itself, but it is important to keep in mind that a migration to TypeScript can lead to unforeseen errors or compatibility issues with other dependencies. Additionally, developers who are unfamiliar with TypeScript may need additional training or time to get up-to-speed with the new codebase.

Commit 410e4564343c56708e737e32003486b34e6b7158

The patch updates the tsconfig.json file by formatting it correctly. There don't appear to be any potential problems with the code changes. However, it's important to check if the formatting of the file follows the team's agreed-upon conventions. The lack of a newline at the end of the file may prompt some linting errors, which the developer should be prepared to address.

Commit e70027501f316c7b87c3e4ccd71260d4705e52a9

This patch updates the package-lock.json file and several dependencies, including "@babel/plugin-proposal-private-property-in-object", "caniuse-lite", "electron-to-chromium", and "html-entities". The update also includes a downgrade of "typescript" from version "5.1.3" to "5.0.4". There do not seem to be any potential problems, as these are common and routine updates to dependencies.

Commit 91d2642dc1301365a6367814c851c5db702e481d

This patch reformats the workflow files and does not introduce any new features or fixes. It makes the YAML files more consistent throughout the project by updating the layout, indentation, and spacing. While this update is helpful for maintaining the codebase, the PR also modifies the issue and pull request templates. These changes could potentially break automation scripts that rely on the old templates' structure. It is crucial to test and ensure their functionality still works before merging this PR.

Commit 8621e9a0c929821c2d406dec4c148017b5bd21b3

The patch contains changes to four files, adding end-of-line characters to each of them. There are no apparent problems with this patch, as adding end-of-line characters can be a good practice in avoiding compatibility issues across different platforms. However, the changes are relatively minor and will not have a significant impact on the codebase or its functionality.

Commit 084632a346fc7120a504616b8d3c9083a5bb775e

It seems that in this patch, only minor changes were made to many different files in the project, such as updating markdown syntax and correcting typos in various documentation files. The patch does not indicate any potential problems or bugs in the code. However, without access to the actual code changes, it is impossible to determine if any functional changes were made that may require additional testing or cause issues with existing features. Overall, it appears to be a routine maintenance patch and the changes seem relatively safe.

Commit cf8021810d3de751d7c4a0350e843d1734476ecf

This patch updates the macOS build instructions in two language files to fix a linting issue by removing a leading whitespace in the header. It is a small and safe change that has no potential problems.

Signed-off-by: Adithya Krishna <adikrish@redhat.com>
@adithyaakrishna
Copy link
Contributor Author

@alabulei1 This PR adds a new workflow i.e. the Build Check to make sure the PR doesnt break the builds before they are merged

Adithya Krishna added 4 commits May 22, 2023 11:03
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
@adithyaakrishna adithyaakrishna marked this pull request as draft May 22, 2023 06:12
Adithya Krishna added 9 commits May 22, 2023 11:49
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Adithya Krishna added 5 commits June 5, 2023 22:27
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
@adithyaakrishna adithyaakrishna marked this pull request as ready for review June 5, 2023 17:42
@adithyaakrishna
Copy link
Contributor Author

@alabulei1 Done, this PR is ready for review 🚀

Adithya Krishna added 3 commits June 6, 2023 11:42
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
@adithyaakrishna adithyaakrishna changed the title [Feat] - Updated Docusaurus to v2.4.1 & Added New Workflows [Feat] - Updated Docusaurus to v2.4.1, Added New Workflows and Migrated to TS Jun 6, 2023
Adithya Krishna added 3 commits June 6, 2023 14:57
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/contribute/release.md Outdated Show resolved Hide resolved
docs/develop/rust/dapr.md Outdated Show resolved Hide resolved
docs/contribute/installer.md Outdated Show resolved Hide resolved
@alabulei1
Copy link
Collaborator

Thanks @adithyaakrishna

Could you tell me why - is better than *?

@adithyaakrishna
Copy link
Contributor Author

Thanks @adithyaakrishna

Could you tell me why - is better than *?

Not particulary, they both are used to denote lists itself. But GitHub Flavoured markdown uses -.

For example, if you click the lists option here, it adds a -, I wanted to make it consistent across the repo, hence used -
Screenshot 2023-06-07 at 9 38 22 AM

Adithya Krishna added 2 commits June 7, 2023 11:01
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
Signed-off-by: Adithya Krishna <adikrish@redhat.com>
@alabulei1
Copy link
Collaborator

LGTM!

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: Package file has duplicate dev dependencies [CI] Deploy CI should be triggered when a PR is raised
2 participants