Skip to content

earthmover_cmd refactor [EDFIAL-466] - #92

Merged
johncmerfeld merged 4 commits into
developmentfrom
fix/earthmover_cmd
Jul 8, 2026
Merged

earthmover_cmd refactor [EDFIAL-466]#92
johncmerfeld merged 4 commits into
developmentfrom
fix/earthmover_cmd

Conversation

@theokaufman

Copy link
Copy Markdown
Collaborator

Fix for 466

  • Define new earthmover_cmd method to encapsulate earthmover calls and logging
  • Refactor earthmover_deps to call earthmover_cmd
  • Refactor earthmover_run to call earthmover_cmd

@theokaufman
theokaufman requested a review from johncmerfeld July 6, 2026 17:36
@amazon-inspector-ohio

Copy link
Copy Markdown

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

@snyk-io-us

snyk-io-us Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@amazon-inspector-ohio

Copy link
Copy Markdown

✅ I finished the code review, and didn't find any security or code quality issues.

if em.stderr:
self.logger.info(f"earthmover stderr: {em.stderr}")

return em

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Wondering if we should just include em.check_returncode() in this function so we don't have to remember to do it after every invocation?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agree that we should. Just updated to include and removed the standalone calls

@johncmerfeld
johncmerfeld merged commit 62273bf into development Jul 8, 2026
4 checks passed
@theokaufman theokaufman mentioned this pull request Jul 10, 2026
@edandylytics edandylytics mentioned this pull request Jul 21, 2026
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.

2 participants