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

Add Winget Error Checking to Scripts #904

Merged
merged 2 commits into from
Sep 11, 2023
Merged

Conversation

JustOptimize
Copy link
Contributor

@JustOptimize JustOptimize commented Sep 8, 2023

Questions

Note: You should commit directly to main for translations of the README.md.

Describe your pull request (edited)

Adds Winget error checking in the Install Open-Shell and Install and Replace Task Manager scripts

This fixes v03-testing > Install Open-shell script error

@github-actions github-actions bot added the playbook Playbook related issues/PRs label Sep 8, 2023
@JustOptimize JustOptimize changed the title replace example.com dns with the winget one replace example.com domain with the winget one Sep 8, 2023
@he3als
Copy link
Contributor

he3als commented Sep 8, 2023

This is good, but I'm wondering if the domain that WinGet is trying to pull from to work will ever get changed or if there's other domains required.

The message should be changed as well to clarify that the user could have internet, but WinGet is blocked.

@JustOptimize
Copy link
Contributor Author

JustOptimize commented Sep 8, 2023

That same domain gets used by the windows store too, I don't know how often they change domains especially because it's the first one that gets pinged

Here is a full list of domains it reaches when starting the install open shell script
image

edit: it's missing cdn.winget.microsoft.com

@he3als
Copy link
Contributor

he3als commented Sep 8, 2023

I think that a better alternative might be simply error checking for WinGet.

@JustOptimize
Copy link
Contributor Author

Working on it 🫡

@JustOptimize
Copy link
Contributor Author

JustOptimize commented Sep 8, 2023

Is checking if the errorlevel is not 0 and if so say something like installation failed enough?

@JustOptimize JustOptimize changed the title replace example.com domain with the winget one Add Winget Error Checking to Scripts Sep 8, 2023
@he3als
Copy link
Contributor

he3als commented Sep 11, 2023

Can you resolve the merge conflicts and add > nul to the Open Shell script please?

@JustOptimize
Copy link
Contributor Author

JustOptimize commented Sep 11, 2023

Can you resolve the merge conflicts

Checking rn

Add > nul to the Open Shell script please?

Already added that with a5ace16

@JustOptimize
Copy link
Contributor Author

To be fair I don't know how to resolve conflicts in a pr

Hiding Winget output like in the "Install and replace Task Manager" script
@JustOptimize
Copy link
Contributor Author

Figured it out 👍🏻

@he3als
Copy link
Contributor

he3als commented Sep 11, 2023

Thank you very much for the PR!

@he3als he3als merged commit 7618eaf into Atlas-OS:dev Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
playbook Playbook related issues/PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants