Skip to content

Fix for register bundle action#124

Merged
guybartal merged 32 commits into
microsoft:mainfrom
guybartal:guybartal/fix-run-command-in-devconrainer
Jun 10, 2025
Merged

Fix for register bundle action#124
guybartal merged 32 commits into
microsoft:mainfrom
guybartal:guybartal/fix-run-command-in-devconrainer

Conversation

@guybartal
Copy link
Copy Markdown
Collaborator

@guybartal guybartal commented Feb 4, 2025

Resolves #122

What is being addressed

  1. Overwrite command.sh when running command via GitHub action
  2. Fix register bundle

How is this addressed

  • Fix for register bundle by using ${{ key }} directly without assigning them to local variable which doesn't work
  • remove hardcoded location value and remove gitref input parameter

guybartal and others added 18 commits February 4, 2025 09:48
@guybartal guybartal marked this pull request as draft February 9, 2025 19:27
@guybartal guybartal marked this pull request as ready for review February 9, 2025 19:33
@guybartal
Copy link
Copy Markdown
Collaborator Author

this PR is blocked and waits for microsoft/AzureTRE#4192 to be released and copied into this repo

@guybartal guybartal changed the title Fix for run command in devcontainer Fix for register bundle action Feb 9, 2025
@marrobi
Copy link
Copy Markdown
Member

marrobi commented Feb 9, 2025

Can you revert the original PR so people aren't blocked from using this repo?

Then can revisit when the mentioned PR is merged?

Thanks.

@tamirkamara
Copy link
Copy Markdown
Contributor

Can you revert the original PR so people aren't blocked from using this repo?

Then can revisit when the mentioned PR is merged?

Thanks.

I've already reverted it

@marrobi
Copy link
Copy Markdown
Member

marrobi commented Feb 12, 2025

@guybartal can this be closed, as once microsoft/AzureTRE#4192 is handled it will come across with the next release?

@guybartal
Copy link
Copy Markdown
Collaborator Author

correct @marrobi

@marrobi marrobi closed this Feb 13, 2025
@guybartal guybartal reopened this Feb 13, 2025
@guybartal
Copy link
Copy Markdown
Collaborator Author

reopening this as this holds a fix for register bundle, but should be merged only after microsoft/AzureTRE#4192 is released and copied to this repo

@guybartal guybartal marked this pull request as draft February 13, 2025 09:12
@guybartal
Copy link
Copy Markdown
Collaborator Author

refactored and waits now for microsoft/AzureTRE#4372 to be merged instead of microsoft/AzureTRE#4192

@guybartal guybartal marked this pull request as ready for review March 3, 2025 10:10
Copy link
Copy Markdown
Member

@marrobi marrobi left a comment

Choose a reason for hiding this comment

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

LGTM, couple of comments, should they be fixed upstream?

Comment thread .github/actions/devcontainer_run_command/action.yml
Comment thread .github/workflows/deploy_tre_reusable.yml
@marrobi
Copy link
Copy Markdown
Member

marrobi commented Mar 19, 2025

@guybartal where are we with this?

@guybartal
Copy link
Copy Markdown
Collaborator Author

@guybartal where are we with this?

Do we want to wait for v0.22.0 to be released with my PR?

@guybartal guybartal enabled auto-merge (squash) April 23, 2025 09:41
@guybartal guybartal disabled auto-merge April 23, 2025 09:41
@guybartal guybartal enabled auto-merge (squash) April 23, 2025 09:41
@matthew-a-dunlap
Copy link
Copy Markdown

matthew-a-dunlap commented Jun 4, 2025

Has there been any traction on getting this pr pulled in? We are facing similar issues using the register action and are looking to use it to resolve a few problems.

@marrobi
Copy link
Copy Markdown
Member

marrobi commented Jun 5, 2025

@guybartal is there any reason that this can't be merged?

@guybartal
Copy link
Copy Markdown
Collaborator Author

There was a weird issue with the linter blocking me from merging, shall I force approve ?

@marrobi
Copy link
Copy Markdown
Member

marrobi commented Jun 5, 2025

That's only for e2e in the main repo.

Linting error looks like is a property in the workflow that's not in the child workflow. Will need fixing. I can take a look if that's the only issue.

@guybartal guybartal merged commit bd2048a into microsoft:main Jun 10, 2025
2 checks passed
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.

.devcontainer command not found

4 participants