-
-
Notifications
You must be signed in to change notification settings - Fork 134
Update atmos describe affected
command
#1267
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
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: 0
🧹 Nitpick comments (1)
pkg/atlantis/atlantis_generate_repo_config_test.go (1)
80-83
: Good fix for path resolution in test environments.This change properly addresses the path resolution issue when tests run in temporary directories. The explicit override ensures the configuration points to the correct fixture location regardless of where the test is executed.
Consider using
filepath.Join
or similar path utilities for better cross-platform compatibility:- atmosConfig.BasePath = "./tests/fixtures/scenarios/complete" + atmosConfig.BasePath = filepath.Join(".", "tests", "fixtures", "scenarios", "complete")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/atlantis/atlantis_generate_repo_config_test.go
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1267 +/- ##
==========================================
+ Coverage 48.40% 49.14% +0.74%
==========================================
Files 233 233
Lines 25505 25497 -8
==========================================
+ Hits 12345 12531 +186
+ Misses 11554 11305 -249
- Partials 1606 1661 +55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 0
🧹 Nitpick comments (3)
website/docs/cli/commands/describe/describe-affected.mdx (3)
396-396
: Typographical: consider using an em dash
To join two clauses or introduce an example, replace the hyphen after "affected" with an em dash for better readability:- `affected` - shows the first (in the processing order) section that was changed. + `affected` — shows the first (in the processing order) section that was changed.🧰 Tools
🪛 LanguageTool
[typographical] ~396-~396: To join two clauses or introduce examples, consider using an em dash.
Context: ...other affected component -affected
- shows the first (in the processing order...(DASH_RULE)
598-598
: Typographical: em dash for definition
Similarly, use an em dash in theaffected_all
bullet for consistency:- `affected_all` - shows all component sections and attributes that were changed + `affected_all` — shows all component sections and attributes that were changed🧰 Tools
🪛 LanguageTool
[typographical] ~598-~598: To join two clauses or introduce examples, consider using an em dash.
Context: ... } ] ``` -affected_all
- shows all component sections and attribu...(DASH_RULE)
600-617
: Align JSON example indentation
In the example output, the indentation of theaffected_all
array items is inconsistent (one item is indented with 12 spaces, the other with 13). Consider normalizing both to two-space indentation within the block for consistency:- "affected_all": [ - "stack.vars", - "stack.settings" - ] + "affected_all": [ + "stack.vars", + "stack.settings" + ]🧰 Tools
🪛 LanguageTool
[style] ~600-~600: Consider shortening or rephrasing this to strengthen your wording.
Context: ...t were changed For example, if you make changes to thevars
andsettings
sections of t...(MAKE_CHANGES)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/docs/cli/commands/describe/describe-affected.mdx
(14 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/describe/describe-affected.mdx
[typographical] ~396-~396: To join two clauses or introduce examples, consider using an em dash.
Context: ...other affected component - affected
- shows the first (in the processing order...
(DASH_RULE)
[typographical] ~598-~598: To join two clauses or introduce examples, consider using an em dash.
Context: ... } ] ``` - affected_all
- shows all component sections and attribu...
(DASH_RULE)
[style] ~600-~600: Consider shortening or rephrasing this to strengthen your wording.
Context: ...t were changed For example, if you make changes to the vars
and settings
sections of t...
(MAKE_CHANGES)
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Lint (golangci)
- GitHub Check: Build (ubuntu-latest, linux)
- GitHub Check: website-deploy-preview
- GitHub Check: Analyze (go)
- GitHub Check: Summary
🔇 Additional comments (4)
website/docs/cli/commands/describe/describe-affected.mdx (4)
203-316
: Readable definition list for flags
The conversion of the flags section from a table to a definition list (<dl>
,<dt>
,<dd>
) enhances clarity and makes it easier to scan flag names and descriptions. Great improvement for maintainability.
343-343
: Document newaffected_all
field in output schema
The JSON schema has been updated to include"affected_all": []
. Ensure the accompanying description below clearly explains its purpose.
619-637
: Example for new component is clear
The scenario demonstrating a newly created component and itsaffected_all
entries is well-written and informative.
679-683
: Clarify exclusion of abstract and disabled components
Good addition to note that abstract (metadata.type
) and disabled (metadata.enabled
) components are excluded. This addresses user expectations directly.
Warning This PR exceeds the recommended limit of 1,000 lines.Large PRs are difficult to review and may be rejected due to their size. Please verify that this PR does not address multiple issues. |
These changes were released in v1.177.0. |
what
atmos describe affected
commandaffected_all
attribute to the output ofatmos describe affected
to show all component sections and attributes that were changedwhy
Improve usage of the
atmos describe affected
command in CI/CD, e.g. GitHub actionsaffected_all
will allow detecting everything that was changed for a component in a stack (including Terraform configurations/components/modules) and ack on the changesFor example, if you make changes to the
vars
andsettings
sections of the componentcomponent-1
in thenonprod
stack, and executeatmos describe affected
, you will get the following result:If you create a new Terraform/Tofu component, configure a new Atmos component
component-1
in thenonprod
stack, and executeatmos describe affected
, you will get the following result:where:
affected
shows that the Atmos component'smetadata
section was changed(since the component is new and the
metadata
section is the first section that Atmos processes)affected_all
shows all the affected sections and attributes:component
- the Terraform component (Terraform configuration) was affected (since it was just added)stack.metadata
- the Atmos component'smetadata
section was changedstack.vars
- the Atmos component'svars
section was changedstack.env
- the Atmos component'senv
section was changedstack.settings
- the Atmos component'ssettings
section was changedSummary by CodeRabbit
New Features
affected_all
, to the output of thedescribe affected
command, providing a list of all changed sections for affected components and stacks.Bug Fixes
Documentation
affected_all
attribute, with revised examples and clarifications.Tests
describe affected
command to verify correct behavior and output.Refactor