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

Removing 'ImageVersion' as an env var as we already use 'IMAGE_VERSION' #2509

Merged
merged 6 commits into from
Jan 27, 2021

Conversation

jmos5156
Copy link
Contributor

Description

Improvement.

See related issue item #2508

This is mainly code clean up in relation to the new install scripts. We are setting two environment variables with the same result but written in two different ways IMAGE_VERSION and ImageVersion. After looking through the code (functions, modules, and scripts) the only place where ImageVersion is used happens to be in the SoftwareReport.Generator. This PR simply changes this to keep to one set on common env vars using the same format.

Related issue:

Check list

  • [*] Related issue / work item is attached
  • Tests are written (if applicable)
  • Documentation is updated (if applicable)
  • [*] Changes are tested and related VM images are successfully generated

@al-cheb
Copy link
Contributor

al-cheb commented Jan 21, 2021

@jmos5156
Copy link
Contributor Author

Hi @al-cheb.
I see what you mean now. I'll update this PR.
However, to make sure I get this right should ImageVersion still be changed to IMAGE_VERSION even though we move it to Initialize-VM.ps1?

IMAGE_VERSION is used throughout the codebase.

ImageVersion is only used in the SoftwareReport Generator script.

@al-cheb
Copy link
Contributor

al-cheb commented Jan 21, 2021

@jmos5156 , It was request from a customer - #345 to add ImageVersion env variable on Windows images. If you delete it it will impact on all builds which use ImageVersion env variable.

@jmos5156
Copy link
Contributor Author

Hi guys wondering if this could be reviewed?

@AlenaSviridenko
Copy link
Contributor

/azp run windows2016, windows2019

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@al-cheb
Copy link
Contributor

al-cheb commented Jan 26, 2021

@jmos5156, Please merge the latest changes from main branch to start an image generation process.

@jmos5156
Copy link
Contributor Author

I have merged the latest changes from the main branch into this PR.

@AlenaSviridenko
Copy link
Contributor

/azp run windows2016, windows2019

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@jmos5156
Copy link
Contributor Author

Are we ok to approve this now?

@AlenaSviridenko
Copy link
Contributor

Merging this one, thanks for contribution!

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.

6 participants