Skip to content

fix: CI workflows bug and linting #109

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

Merged
merged 6 commits into from
Jun 15, 2023

Conversation

adithyaakrishna
Copy link
Contributor

Description:

  • Fixes Lint CI
  • Updated node version in CI to use 16.14.0
  • Updated spacing in workflows

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

alabulei1 commented Jun 15, 2023

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


The patch fixes various linting issues, updates Node.js in the deployment workflow, and fixes a CodeQL bug by replacing string replace method with a regex. While there are no potential problems with the updates and fixes themselves, there are some issues that could arise if they are not carefully managed. These include a potentially long timeout for the lint workflow, possible dependency issues or test-breaking in future versions due to the installation of dependencies without specifying package versions, and limitations on the validation filter regex used in the lint workflow. Additionally, there is a lack of detail in the pull request title and potential confusion over whether there are changes beyond the EOL update. Overall, these changes are minor and targeted but would benefit from clarification and further context.

Details

Commit 21d632917542c8f47e6dbff37d9b9396912cc487

Key Changes:

  • Added linting and fixed a bug in both dependency-review and codeql-analysis workflows.
  • Included a new lint job that checks code format using Prettier. It also installs relevant dependencies and sets up Node JS in the workflow.

Potential Problems:

  • The lint workflow has a potentially long timeout of 30 minutes, which could lead to unnecessary delays.
  • The lint job installs dependencies and sets up Node JS without specifying package versions, which could raise dependency issues or break tests in future versions.
  • The filter regex for validating markdown in the lint-workflow is set to only validate docs/*.md files. If there are files with different extensions or in different directories that need to be checked, this will need to be updated.
  • The lint job also does not have a final step of committing and pushing the changes like the previous workflows. This means the lint checking will not update the repository but will halt on the linting results.

Commit 83bf29af2cc515a32546b6d1d9434d3263f80a2f

The patch updates the version of Node.js used in the deployment workflow from 16.0 to 16.14.0. There does not seem to be any potential problems with this change. It is always good to keep the dependencies up to date as it can bring bug fixes, security enhancements, and better performance.

Commit e064ee208e6c780cc3be3eba510d8c9a7d7ff2e1

This patch contains 833 insertions and 834 deletions. It fixes linting issues in several files. There are no potential problems.

Commit e3cfb384f872a1ff5f4a39e1ee99710ab6657ec7

The patch fixes linting issues across various files. There are no potential problems identified in the provided patch.

Commit 4789ca8f7852ce70713b6ede6c8252aa089ff175

The patch adds an End of Line (EOL) to the file and fixes a missing newline issue. The change seems to be minor and has low risk to break anything in the software.

However, it is worth noting that the patch only affects one file, and it is not clear if the pull request has any other changes beyond this patch. Additionally, because the pull request is titled "fix: CI workflows bug and linting," it would be good to have a more detailed explanation of the changes made.

Commit 6dfc4280bbf7e1d7c01dafad5950f73c5e7a2baf

The patch fixes a CodeQL bug by replacing a string replace method with a regular expression. Specifically, it replaces all occurrences of "[" and "]" with an empty string. The patch seems to be small and targeted. There are no potential problems or implications that we can discern from the patch alone. However, we need more context about the changes and whether it affects any other functionality in the codebase.

Adithya Krishna added 4 commits June 15, 2023 10:23
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]>
Signed-off-by: Adithya Krishna <[email protected]>
@adithyaakrishna
Copy link
Contributor Author

CI Passes. SHIP IT 🚀

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.

2 participants