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.

Sign up for GitHub

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

Jump to bottom

fix: clean up naming of things - avm/res/compute/image #2905

Closed
wants to merge 13 commits into from
Closed

fix: clean up naming of things - avm/res/compute/image #2905

wants to merge 13 commits into from

Conversation

tony-box
Copy link
Contributor

@tony-box tony-box commented Aug 1, 2024 •
edited
Loading

Description

  • Cleaned up naming of files and parameters for better clarity

Pipeline Reference

Pipeline
avm.res.compute.image

Type of Change

  • Update to CI Environment or utilities (Non-module affecting changes)
  • Azure Verified Module updates:
    • Bugfix containing backwards-compatible bug fixes, and I have NOT bumped the MAJOR or MINOR version in version.json:
      • Someone has opened a bug report issue, and I have included "Closes #{bug_report_issue_number}" in the PR description.
      • The bug was found by the module author, and no one has opened an issue to report it yet.
    • Feature update backwards compatible feature updates, and I have bumped the MINOR version in version.json.
    • Breaking changes and I have bumped the MAJOR version in version.json.
    • Update to documentation

Checklist

  • I'm sure there are no other open Pull Requests for the same update/change
  • I have run Set-AVMModule locally to generate the supporting module files.
  • My corresponding pipelines / checks run clean and green without any errors or warnings

@tony-box tony-box requested review from a team as code owners August 1, 2024 13:45
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Triage 🔍 Maintainers need to triage still Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue labels Aug 1, 2024
AlexanderSehr
AlexanderSehr reviewed Aug 1, 2024
View reviewed changes
avm/res/compute/image/tests/e2e/max/dependencies.bicep
resource diskEncryptionSet 'Microsoft.Compute/diskEncryptionSets@2022-07-02' = {
module waitForDeployment 'main.wait.bicep' = {
dependsOn: [keyVaultKeyCryptoRole]
scope: resourceGroup()
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is running in the same scope as everything else in this template - why is in in a seperate file I wonder :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I still wonder @tony-box 😄

Tony Box and others added 5 commits August 6, 2024 11:31
Merge branch 'main' into fix/compute-image
e9118fb
@tony-box @AlexanderSehr
remove comments
51ac2ed 
Co-authored-by: Alexander Sehr <ASehr@hotmail.de>
@tony-box @AlexanderSehr
remove tags
d013321 
Co-authored-by: Alexander Sehr <ASehr@hotmail.de>
clean up of wait time stuff
3ccf51f
update readme
e1eda5c
@tony-box tony-box requested a review from AlexanderSehr August 6, 2024 18:21
@matebarabas matebarabas changed the title fix: clean up naming of things fix: clean up naming of things - avm/res/compute/image Aug 6, 2024
@matebarabas matebarabas added Class: Resource Module 📦 This is a resource module Needs: Core Team 🧞 This item needs the AVM Core Team to review it and removed Needs: Triage 🔍 Maintainers need to triage still labels Aug 6, 2024
@eriqua eriqua added the Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author label Sep 27, 2024
@ReneHezser
Copy link
Contributor

ReneHezser commented Oct 4, 2024 •
edited
Loading

@tony-box
We have added examples for Bicep parameter files to the Readme. This has been applied to all published modules but needs to be done for PRs as well. Can you please update your branch and run the Set-AVMModule utility as detailed here. It is required for the validation pipeline to succeed and the contribution to be published.

Please reach out if any support is needed.

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author label Oct 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs: Attention 👋 Reply has been added to issue, maintainer to review label Oct 4, 2024
@eriqua eriqua added Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author and removed Needs: Attention 👋 Reply has been added to issue, maintainer to review labels Oct 8, 2024
AlexanderSehr
AlexanderSehr reviewed Nov 8, 2024
View reviewed changes
avm/res/compute/image/tests/e2e/defaults/dependencies.bicep Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note, the main.json templates must be re-build with the latest Bicep version 0.31.34 as this version (running on the agents) renders a different main.json file then previous versions.

The steps to execute would be

  • Upgrade your Bicep CLI version to 0.31.34 (e.g. via choco upgrade bicep if choco is used)
  • Re-run the Set-AVMModule script on your module. E.g. via Set-AVMModule -ModuleFolderPath '.\avm\res\automation\automation-account\' -SkipFileAndFolderSetup -SkipReadMe -Recurse
  • Push the changes
  • Re-run the tests to ensure everything works.

If the static tests continue to fail for you, please note that there is currently presumably another new bug which affects some of our modules (ref).

@jtracey93
Copy link
Contributor

Hey @tony-box ,

Firstly, thanks for your work on this PR!

We have made some changes to the AVM CI, detailed below, which means we need you to update your fork to pull in these latest changes and re-run your tests to show they still are passing prior to approving and merging this PR, as we don't and it fails once merged the publishing of your module will fail and will be blocked going forward until the test pass again via additional PRs.

Changes to CI That Have Been Made That You Need To Take Action On

Any questions reach out to the AVM Core Team by tagging us in your PR here or internally via Teams

Thanks

Jack (AVM Core Team)

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs: Attention 👋 Reply has been added to issue, maintainer to review and removed Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author labels Nov 15, 2024
@eriqua eriqua added Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author and removed Needs: Attention 👋 Reply has been added to issue, maintainer to review labels Dec 14, 2024
@eriqua
Copy link
Contributor

eriqua commented Dec 14, 2024

Hey @tony-box is this still active?

@microsoft-github-policy-service microsoft-github-policy-service bot added the Status: No Recent Activity 💤 When an issue/PR has not been modified for X amount of days label Dec 18, 2024
@microsoft-github-policy-service Microsoft GitHub Policy Service
Copy link

Important

@tony-box, this issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

Tip

To prevent further actions to take effect, one of the following conditions must be met:

  • The author must respond in a comment within 3 days of this comment.
  • The "Status: No Recent Activity 💤" label must be removed.
  • If applicable, the "Status: Long Term ⏳" or the "Needs: Module Owner 📣" label must be added.

@AlexanderSehr AlexanderSehr mentioned this pull request Dec 30, 2024
Merged
8 tasks
@AlexanderSehr
Copy link
Contributor

Closing in favor of rebased PR #4105.
Note @tony-box, this will still list you as the author of the changes. Just rebased it to get it merged 😉

AlexanderSehr added a commit that referenced this pull request Dec 30, 2024
@AlexanderSehr @tony-box @eriqua
fix: Compute-Image- Clean up naming of things (#4105)
6221280 
## Description

Rebase of #2905 to
conclude PR

Original description:
> Cleaned up naming of files and parameters for better clarity

## Pipeline Reference

<!-- Insert your Pipeline Status Badge below -->

| Pipeline |
| -------- |

[![avm.res.compute.image](https://github.com/AlexanderSehr/bicep-registry-modules/actions/workflows/avm.res.compute.image.yml/badge.svg?branch=users%2Falsehr%2Ftony-box_fix%2Fcompute-image_rebase&event=workflow_dispatch)](https://github.com/AlexanderSehr/bicep-registry-modules/actions/workflows/avm.res.compute.image.yml)

## Type of Change

<!-- Use the checkboxes [x] on the options that are relevant. -->

- [ ] Update to CI Environment or utilities (Non-module affecting
changes)
- [x] Azure Verified Module updates:
- [ ] Bugfix containing backwards-compatible bug fixes, and I have NOT
bumped the MAJOR or MINOR version in `version.json`:
- [ ] Someone has opened a bug report issue, and I have included "Closes
#{bug_report_issue_number}" in the PR description.
- [ ] The bug was found by the module author, and no one has opened an
issue to report it yet.
- [ ] Feature update backwards compatible feature updates, and I have
bumped the MINOR version in `version.json`.
- [ ] Breaking changes and I have bumped the MAJOR version in
`version.json`.
  - [ ] Update to documentation

---------

Co-authored-by: Tony Box <tobox@microsoft.com>
Co-authored-by: Tony Box <tonybox@gmail.com>
Co-authored-by: Erika Gressi <56914614+eriqua@users.noreply.github.com>
@tony-box tony-box deleted the fix/compute-image branch December 30, 2024 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexanderSehr AlexanderSehr AlexanderSehr left review comments

Assignees

@tony-box tony-box

Labels
Class: Resource Module 📦 This is a resource module Needs: Author Feedback 👂 Awaiting feedback from the issue/PR author Needs: Core Team 🧞 This item needs the AVM Core Team to review it Status: No Recent Activity 💤 When an issue/PR has not been modified for X amount of days Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@tony-box @ReneHezser @jtracey93 @eriqua @AlexanderSehr @matebarabas