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

[Go] Refactor the codebase of embed-code-go #43

Open
olena-zmiiova opened this issue Sep 2, 2024 · 3 comments
Open

[Go] Refactor the codebase of embed-code-go #43

olena-zmiiova opened this issue Sep 2, 2024 · 3 comments
Assignees

Comments

@olena-zmiiova
Copy link

olena-zmiiova commented Sep 2, 2024

As of now, we feel that current Go codebase is mature enough to be reviewed by someone, who hasn't been originally involved into the project. This is needed to ensure the longevity of the code for the years to come — as we expect to use Go-based tooling for a long time.

In the scope of this issue, the following steps should be performed:

  1. The review of the existing source code.
  2. Identification of issues.
  3. Addressing the issues (todo: avoid massive changesets, go small).
  4. (if possible) Configuring and enabling the automatic linters to prevent the issues in the future.
  5. (optional) Forming a page on wiki on the code style.

The last point needs to be discussed prior to its execution.

@olena-zmiiova olena-zmiiova self-assigned this Sep 2, 2024
@armiol armiol changed the title [Go] Refactor the codebase of embed-code-go [Go] Refactor the codebase of embed-code-go Sep 2, 2024
@armiol
Copy link
Collaborator

armiol commented Sep 2, 2024

@olena-zmiiova @dmytro-kashcheiev I have updated the text of the issue. FYI.

@SpineEventEngine SpineEventEngine deleted a comment from JigarJoshi04 Sep 2, 2024
@SpineEventEngine SpineEventEngine deleted a comment from JigarJoshi04 Sep 2, 2024
@olena-zmiiova
Copy link
Author

After the code analyses, I came out with the following plan of refactoring. Some of the points might be complex, and I'll create other GH issues to complete them.

Code:

  1. Function bodies and logic should be easier to read and understand. It should be done package by package.
  2. Errors:
    • We don’t need a panic in every case.
    • Consider returning an error instead of just a message for validation.
    • Validate error processing and add where it’s needed.
  3. Entrypoint arguments:
    • Update validation code.
    • Use another naming convention (including fields in config .yaml files).
  4. Logging:
    • Add logs to make what is happening clear and easier to debug.
    • Add output to a user on a successful run.

Tests:

  1. Test files should be placed near project files.
  2. Consider using ginkgo and/or gomega libraries.

Documentation:

  1. Validate and update project docs/readmes.
  2. Add documentation for packages where missed.
  3. Add name reference in docs.

CI:

  1. Configure run of golangci-lint on PRs.
  2. Configure docs generating with godoc (optional).

@olena-zmiiova
Copy link
Author

olena-zmiiova commented Sep 26, 2024

In the scope of an issue, the list of PRs was merged:

The main task of refactoring can be considered as resolved, but here are the possible points to improve the embed-code codebase:

  • Improve logging.
  • Configure godoc generation.
  • GitHub actions should run as a chain: if there are no linter issues and all tests pass, the code should be built into binaries. Currently, the embed-code is built every time on PRs into embed-code-go branch, even if tests or linter has failed.
  • Add more test cases to improve the test coverage, and consider using parameterized tests.

Important

Do not forget to rename fields in JxBrowser-Docs/_hugo-site/embed-code-config.yml file before updating the binaries in JxBrowser-Docs. Please, refer to README.md for the new namings.

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

No branches or pull requests

2 participants