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(ci): adds more CI validations, introduces semantic release (#74) #75

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

ygrishajev
Copy link
Contributor

@ygrishajev ygrishajev commented Apr 15, 2024

Feature: Enhance CI Validations and Introduce Semantic Release

This pull request introduces several enhancements to our Continuous Integration (CI) pipeline and implements Semantic Release for automated version management and changelog generation. These changes aim to bolster code quality and streamline our release process.

Key Changes

  1. ESLint Integration

    • Added an ESLint check as a pre-commit hook to ensure that any commits comply with our coding standards.
    • Integrated ESLint checks into our CI pipeline to validate code style and catch issues early in the review process.
  2. Commit Linting

    • Implemented commit linting both as a pre-commit hook and within the CI to enforce our commit message guidelines. This ensures that all commit messages are consistent and follow the Conventional Commits specification.
  3. Semantic Release

    • Adopted Semantic Release to automate the versioning and changelog generation process. This tool leverages information from commit messages to determine version bumps and generate comprehensive changelogs automatically upon merging changes into the main branch.
  4. Code Coverage Reporting

    • Enhanced our CI pipeline to include the upload of code coverage reports to Codecov. This allows us to monitor and maintain code quality effectively by reviewing coverage metrics for each pull request.

Benefits

  • Improved Code Quality: By enforcing strict linting rules before commits and during CI, we reduce the likelihood of errors and ensure code consistency across the project.
  • Automated Releases: Semantic Release automates the release cycle, reduces the manual effort required for version management, and ensures that our changelog is always up-to-date.
  • Consistency in Commits: Enforcing commit message guidelines helps maintain a clean and navigable project history, facilitating easier troubleshooting and review processes.
  • Transparent Code Quality Metrics: With code coverage results now visible on Codecov, contributors and reviewers can directly observe and address any areas lacking sufficient testing.

Additional Notes

  • Please ensure to install the necessary hooks by running npm install after pulling these changes.
  • Review the updated GitHub Actions configurations to understand the new CI checks.

Associated Issues

@ygrishajev ygrishajev marked this pull request as ready for review April 15, 2024 08:34
@ygrishajev ygrishajev force-pushed the feature/ci branch 9 times, most recently from b73ad0a to b0c356f Compare April 15, 2024 09:10
@ygrishajev ygrishajev requested review from baktun14 and Redm4x April 15, 2024 09:11
@codecov-commenter
Copy link

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@ygrishajev
Copy link
Contributor Author

as discussed with @baktun14 we also would need to properly handle commit linting for squash merge. On the surface solution is to take PR title as a squash commit and lint it to allow merge

@ygrishajev
Copy link
Contributor Author

@akash-network/cloudmos I suggest that we split checks into dedicated jobs:

  1. linting
  2. tests
  3. commit linting

Then protect merge in to main the same way it's done in https://github.com/akash-network/akash-api/settings/branch_protection_rules/34186868

This way those would run independently addressing a scenario like when you're pushing commits that wouldn't pass linting but would pass tests.

However there's an issue with this anyway. There is no way to conditionally allow a certain type of merge (squash/rebase/merge). So to address any case a feature PR would need to have valid commit messages AND the title. This would require to come up with proper commit names as you go and might kill the purpose of squash merge.

Let's try to work out what we'd like this to be

@baktun14
Copy link
Contributor

@akash-network/cloudmos I suggest that we split checks into dedicated jobs:

  1. linting
  2. tests
  3. commit linting

Then protect merge in to main the same way it's done in https://github.com/akash-network/akash-api/settings/branch_protection_rules/34186868

This way those would run independently addressing a scenario like when you're pushing commits that wouldn't pass linting but would pass tests.

However there's an issue with this anyway. There is no way to conditionally allow a certain type of merge (squash/rebase/merge). So to address any case a feature PR would need to have valid commit messages AND the title. This would require to come up with proper commit names as you go and might kill the purpose of squash merge.

Let's try to work out what we'd like this to be

Yea it would be a bit annoying to have to have valid commits AND title. Not the biggest deal, but I think since we've been doing squash merges from the beginning, it make sense to only apply the pr title contraint and suggest for the commits if you plan to do a normal merge.

@ygrishajev
Copy link
Contributor Author

Yea it would be a bit annoying to have to have valid commits AND title. Not the biggest deal, but I think since we've been doing squash merges from the beginning, it make sense to only apply the pr title contraint and suggest for the commits if you plan to do a normal merge.

So one another way to do this may be to ONLY allow squash commits (not sure how would that work with just 1 in pr) and validate the title.

And another one (a mild one) would be not to fail a check but to post a warning.

I'd prob try going with the more strict one.

@baktun14
Copy link
Contributor

Yea it would be a bit annoying to have to have valid commits AND title. Not the biggest deal, but I think since we've been doing squash merges from the beginning, it make sense to only apply the pr title contraint and suggest for the commits if you plan to do a normal merge.

So one another way to do this may be to ONLY allow squash commits (not sure how would that work with just 1 in pr) and validate the title.

And another one (a mild one) would be not to fail a check but to post a warning.

I'd prob try going with the more strict one.

Yea makes sense to me

@troian
Copy link
Member

troian commented Apr 16, 2024

  1. set the following in the project settings. that way commit will be clean
image 2. it is still good practice to format all commits within PR with semantics of conventional commits

@ygrishajev ygrishajev force-pushed the feature/ci branch 9 times, most recently from a17bd97 to 9a49a5b Compare April 16, 2024 10:06
@ygrishajev ygrishajev force-pushed the feature/ci branch 25 times, most recently from ada3bb2 to 7d48a7f Compare April 17, 2024 12:40
Also:
- Uses semantic release
- Adds checks on before commit and in CI: eslint and commit lint
@ygrishajev ygrishajev merged commit 45fa556 into main Apr 22, 2024
3 checks passed
@ygrishajev ygrishajev deleted the feature/ci branch April 22, 2024 15:17
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.

Implement Commit Convention and Release Automation
4 participants