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

minor mashtree improvements #142

Merged
merged 2 commits into from
Aug 7, 2023
Merged

minor mashtree improvements #142

merged 2 commits into from
Aug 7, 2023

Conversation

kapsakcj
Copy link
Contributor

@kapsakcj kapsakcj commented Aug 3, 2023

Setting as draft until I test in Terra. Just noticed that the user cannot alter the docker image used for mashtree and there is no docker output string.

🛠️ Changes Being Made

  • expose optional string input docker for mashtree task
  • added new String output docker for mashtree task & mashtree_docker String output for the mashtree workflow

🧠 Context and Rationale

More flexibility

📋 Workflow/Task Steps

Inputs

Outputs

🧪 Testing

Locally

Did not test locally, this is a simple change

Terra

Tested successfully in Terra. Was able to pass in an optional docker input string, which was used correctly. Also produced the correct output string/column for mashtree_docker

🔬 Quality checks

Pull Request (PR) checklist:

  • Include a description of what is in this pull request in this message.
  • The workflow/task has been tested locally and on Terra
  • The CI/CD has been adjusted and tests are passing
  • Everything follows the style guide

@kapsakcj kapsakcj changed the title expose optional string input "docker" for mashtree task minor mashtree improvements Aug 3, 2023
@kapsakcj kapsakcj marked this pull request as ready for review August 3, 2023 20:43
Copy link
Member

@sage-wright sage-wright left a comment

Choose a reason for hiding this comment

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

Code looks good; running a test here and will approve once completed successfullyl

Copy link
Member

@sage-wright sage-wright left a comment

Choose a reason for hiding this comment

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

Tests succeeded. Now merging to main!

@sage-wright sage-wright merged commit 83f7e39 into main Aug 7, 2023
5 checks passed
@kapsakcj kapsakcj deleted the cjk-mashtree-docker branch August 9, 2023 15:37
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