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

Added check for attemptToFetchMajorUpgrade #283

Conversation

colorenz
Copy link
Contributor

Hey,

the variable attemptToFetchMajorUpgrade has not been used yet.

I have added a check for attemptToFetchMajorUpgrade. I hope that was correct. My tests were successful. I hope its not to dirty :D

Best Regards

The variable attemptToFetchMajorUpgrade has not been used yet. 

I have added a check for attemptToFetchMajorUpgrade. I hope that was correct. My tests were successful. I hope its not to dirty :D
@erikng
Copy link
Member

erikng commented Nov 21, 2021

I don't think this logic is entirely correct. If the value is false, the device will hit the "else" block and then try to run the minor updates.

I think the check you're adding needs to be at the top of the "if statement" and to log if it's false and then return from the entire function so no other logic runs.

@colorenz
Copy link
Contributor Author

Yes, that's what I thought at first.

Maybe I'm wrong, but I think my way would have the possibility to distribute a major and a minor update to different OS versions. Like described in the examples.
https://github.com/macadmins/nudge/wiki/targetedOSVersionsRule#real-world-example-2

  • You can update BigSur 11.6.1 to 12.0.1 -> it goes to the major if block Utils().requireMajorUpgrade() && attemptToFetchMajorUpgrade
  • You can update Monterey 12.0 to 12.0.1 -> it goes to the minor "else" block

Best Regards

@erikng
Copy link
Member

erikng commented Nov 22, 2021

Thanks for doing this but I'm going to do it slightly differently: #284

@erikng erikng closed this Nov 22, 2021
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.

2 participants