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

Fix configuration file #138

Merged
merged 5 commits into from
Dec 10, 2024
Merged

Conversation

Gijsreyn
Copy link
Contributor

@Gijsreyn Gijsreyn commented Dec 9, 2024


Addresses issue #137

Microsoft Reviewers: Open in CodeFlow

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Dec 9, 2024

@Trenly and @ryfu-msft I think there was one small step missing in the configuration file inside this repository. This should fix it.

@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Attention This work item needs to be reviewed by a member of the core team label Dec 9, 2024
@Trenly
Copy link
Contributor

Trenly commented Dec 9, 2024

@Trenly and @ryfu-msft I think there was one small step missing in the configuration file inside this repository. This should fix it.

The configuration was built presuming the user was already operating inside the cloned repository, as would be the most common workflow when using DevHome

@Gijsreyn
Copy link
Contributor Author

Gijsreyn commented Dec 9, 2024

@Trenly and @ryfu-msft I think there was one small step missing in the configuration file inside this repository. This should fix it.

The configuration was built presuming the user was already operating inside the cloned repository, as would be the most common workflow when using DevHome

That was my assumption as well, but for fresh boxes, perhaps it is better to include it?

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Attention This work item needs to be reviewed by a member of the core team label Dec 9, 2024
Copy link
Contributor

@ryfu-msft ryfu-msft left a comment

Choose a reason for hiding this comment

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

I'd have to agree with Trenly here as well. In my mind the most common workflow would be to start by cloning this repo, then running winget configure on the configuration file. Your change will cause it to clone the same repo again. I think it is probably best to leave out the git clone part as that is not really needed within the context of this repo.

@microsoft-github-policy-service microsoft-github-policy-service bot removed Changes-Requested Changes Requested Needs-Author-Feedback This needs a response from the author. labels Dec 10, 2024
@Gijsreyn
Copy link
Contributor Author

Thanks for the sharing and interesting thoughts. I would have assumed the GitClone resource would test for it. If it is already present, it applies the configuration successfully without additional cloning (depending on where the user executes it, of course).

Regardless, I have removed the GitClone for now. I have fixed up the cloning because it was pointing towards winget-pkgs. I have added the PowerShell requirement.

@Gijsreyn Gijsreyn requested a review from ryfu-msft December 10, 2024 05:37
@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@ryfu-msft ryfu-msft merged commit 2aee9b7 into microsoft:main Dec 10, 2024
5 checks passed
@Gijsreyn Gijsreyn deleted the fix-configuration-apply branch December 11, 2024 13:03
@Gijsreyn Gijsreyn mentioned this pull request Dec 28, 2024
2 tasks
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.

3 participants