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

feat: Updates base image and adds features the nice way, allows local testing #1406

Merged
merged 12 commits into from
Mar 27, 2024

Conversation

ReneHezser
Copy link
Contributor

Description

Updates devcontainer

  • base image
  • installs features via "features"
  • allows local GitHub action testing

Pipeline Reference

not needed

Type of Change

  • Update to CI Environment or utilities (Non-module effecting 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

@ReneHezser ReneHezser requested review from a team as code owners March 25, 2024 10:39
@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 Mar 25, 2024
@ChrisSidebotham
Copy link
Contributor

@segraef - Are you happy with this?

@segraef
Copy link
Contributor

segraef commented Mar 25, 2024

@segraef - Are you happy with this?

Yup, much more cleaner way, thanks @ReneHezser.

@matebarabas matebarabas added Type: Hygiene 🧹 things related to testing, issue triage etc. and removed Needs: Triage 🔍 Maintainers need to triage still labels Mar 26, 2024
AlexanderSehr
AlexanderSehr previously approved these changes Mar 26, 2024
Copy link
Contributor

@AlexanderSehr AlexanderSehr left a comment

Choose a reason for hiding this comment

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

Thanks for reviewing @segraef, will approve on that basis.
And of course thank you @ReneHezser for the contribution 💪

@AlexanderSehr
Copy link
Contributor

Dang, prettier takes offense. Very likely because of the file ending:
image

@ReneHezser could you take care of this?

@ReneHezser
Copy link
Contributor Author

ReneHezser commented Mar 26, 2024

Dang, prettier takes offense. Very likely because of the file ending: image

@ReneHezser could you take care of this?

I don't get it. I ran prettier and the file has an empty line at the end. I checked that one in, but the action still complains ¯\(ツ)

AlexanderSehr
AlexanderSehr previously approved these changes Mar 26, 2024
@AlexanderSehr
Copy link
Contributor

@ChrisSidebotham, I think you know more about prettier than me 😄 Would you happen to have any advice? Maybe we can just bypass it by taking the workflow out of the policy as we may or may not retire it anyhow.

Note

The "Type: AVM 🅰️ ✌️ Ⓜ️" label was added as per ITA08BCP.

@ChrisSidebotham
Copy link
Contributor

From my understanding this happens when your editor switches from CRLF to LF and prettier doesn't like it, but we never managed to confirm that

@AlexanderSehr
Copy link
Contributor

Closing and Re-opening to re-trigger the workflow validation...

@AlexanderSehr
Copy link
Contributor

AlexanderSehr commented Mar 27, 2024

I'm impressed. Even by removing the workflow requirement and re-opening the PR the check prevails. @ReneHezser at this point the only solution I'm seeing is to open a new PR from the same branch you opened this one from. Could you give that a shot?

@ReneHezser
Copy link
Contributor Author

I am testing Prettier and its discomfort with this file anyway. CRLF is not the problem.

@AlexanderSehr AlexanderSehr changed the title updates base image and adds features the nice way, allows local testing feat: Updates base image and adds features the nice way, allows local testing Mar 27, 2024
@AlexanderSehr AlexanderSehr merged commit 54fdcb3 into Azure:main Mar 27, 2024
9 of 10 checks passed
@ReneHezser ReneHezser deleted the devcontainer-update branch March 27, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: AVM 🅰️ ✌️ Ⓜ️ This is an AVM related issue Type: Hygiene 🧹 things related to testing, issue triage etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants