Skip to content

[Internal] Update Contributing guide #4404

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

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

Conversation

mgyucht
Copy link
Contributor

@mgyucht mgyucht commented Jan 16, 2025

Changes

Update contributing guide to refer to NEXT_CHANGELOG.md.

Tests

  • make test run locally
  • relevant change in docs/ folder
  • covered with integration tests in internal/acceptance
  • using Go SDK
  • using TF Plugin Framework

@mgyucht mgyucht requested review from a team as code owners January 16, 2025 13:24
@mgyucht mgyucht requested review from rauchy and removed request for a team January 16, 2025 13:24
@mgyucht mgyucht temporarily deployed to test-trigger-is January 16, 2025 13:24 — with GitHub Actions Inactive
@mgyucht mgyucht temporarily deployed to test-trigger-is January 16, 2025 13:24 — with GitHub Actions Inactive
CONTRIBUTING.md Outdated
@@ -51,6 +51,8 @@ Code contributions—bug fixes, new development, test improvement — all follow
git commit -m "commit message here"
```

1. Document your changes in the `NEXT_CHANGELOG.md` under the appropriate heading.
Copy link
Contributor

Choose a reason for hiding this comment

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

This may lead to conflicts when we have many PRs in parallel

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is true. It's a tradeoff of this approach, but I think it should be acceptable as long as the test loop can be made faster. We are going to build a plan for this, involving setting up a test cache where we run integration tests to make sure that commits that don't affect code (like docs changes or changelog updates) don't need to reexecute tests.

Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4404
  • Commit SHA: 82e17c1480fe6b28804a21973d12fdac5fbef86b

Checks will be approved automatically on success.

@nkvuong
Copy link
Contributor

nkvuong commented Feb 25, 2025

@mgyucht @alexott should we also update PULL_REQUEST_TEMPLATE.md with instructions about NEXT_CHANGELOG?

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.

4 participants