-
-
Notifications
You must be signed in to change notification settings - Fork 0
Use atmos instead of makefile #37
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
Conversation
WalkthroughAdds Atmos configuration and ignores its local state, removes build-harness and some Makefile targets, and expands documentation (README and module docs). No runtime code changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Repo as Repo
participant Atmos as Atmos CLI
participant TF as Terraform
participant Tests as Terratest
Note over Repo,Atmos: New workflow with atmos.yaml
Dev->>Repo: Pull repo
Dev->>Atmos: atmos run <cmd>
Atmos->>TF: Init/Plan/Apply (per config)
TF-->>Atmos: Results
alt tests invoked
Atmos->>Tests: Execute test suite
Tests-->>Atmos: Test results
end
Atmos-->>Dev: Output and status
sequenceDiagram
autonumber
actor Dev as Developer
participant Make as Makefile
participant Tests as test/run.sh
Note over Make,Tests: Removed targets
Dev->>Make: make all / make test
Make--x Dev: Targets removed (no-op)
Note right of Dev: Use Atmos or direct Terraform instead
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Important Do not edit the Please update the Could you fix it @goruha? 🙏 |
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
141-214
: Docs section duplicated; unify to one source of truthThe full module docs appear twice in this README. This invites drift and conflicting guidance. Keep a single autogenerated block.
Suggested change: remove the later duplicate block (lines 246–318).
-<!-- markdownlint-disable --> -## Requirements -... -<!-- markdownlint-restore --> +<!-- (removed duplicate docs block; keep a single autogenerated section) -->Also applies to: 246-318
17-23
: Update generation notice to reflect Atmos workflowThe notice still references build-harness. Update to the new generation method to prevent contributor confusion.
- ** This file was automatically generated by the `cloudposse/build-harness`. - ** 1) Make all changes to `README.yaml` - ** 2) Run `make init` (you only need to do this once) - ** 3) Run`make readme` to rebuild this file. + ** This file is automatically generated. + ** 1) Make all changes to `README.yaml` + ** 2) Run `atmos docs generate` (or the configured pre-commit hook) + ** 3) Commit the regenerated README.
🧹 Nitpick comments (6)
.gitignore (2)
10-10
: Prefer directory-specific ignore for Atmos cacheUse a trailing slash to unambiguously ignore the directory and avoid accidental file matches.
-.atmos +.atmos/
1-12
: Mirror .atmos ignore to .dockerignore per file headerHeader says entries “below here should also be in .dockerignore”. Ensure
.dockerignore
includes.atmos/
to keep local Atmos state out of images.atmos.yaml (3)
11-11
: Add newline at EOF (yamllint failure)YAML lints flag “no newline at end of file”. Add a trailing newline.
- https://raw.githubusercontent.com/cloudposse-terraform-components/.github/refs/heads/main/.github/atmos/terraform-component.yaml +
9-11
: Pin imported config to immutable ref to avoid driftImporting from
refs/heads/main
risks silent behavior changes. Prefer a tag or commit SHA; alternatively vendor this file.Example options:
- Use a tag:
.../.github/refs/tags/vX.Y.Z/...
- Use a commit:
.../.github/<commit_sha>/.github/atmos/terraform-component.yaml
9-11
: Docs/test link mismatch: imported file vs README linkThis repo imports
terraform-component.yaml
, while README links toterraform-module.yaml
for test commands. Confirm the file you import actually definesatmos test
commands referenced in the docs, or align the import and docs.README.md (1)
413-426
: Verify Atmos test commands are defined in imported configThe docs instruct
atmos test run/clean
and link to aterraform-module.yaml
, butatmos.yaml
importsterraform-component.yaml
. Ensure the imported file actually defines these custom commands or adjust the link/instructions accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore
(1 hunks)Makefile
(0 hunks)README.md
(3 hunks)atmos.yaml
(1 hunks)src/README.md
(2 hunks)
💤 Files with no reviewable changes (1)
- Makefile
🧰 Additional context used
🪛 YAMLlint (1.37.1)
atmos.yaml
[error] 11-11: no new line character at the end of file
(new-line-at-end-of-file)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
These changes were released in v1.535.3. |
what
why
Summary by CodeRabbit
Documentation
Chores