issue #5409 support for amazonlinux, tarball reinstall / upgrade and clean up #5420
Conversation
DarwinJS
requested review from
TravisEz13,
adityapatwardhan and
daxian-dbw
as code owners
| if [[ "$repobased" == true ]] ; then | ||
| echo "*** NOTE: Run your regular package manager update cycle to update PowerShell Core" | ||
| else | ||
| echo "*** NOTE: Re-run this script to update PowerShell Core" |
There was a problem hiding this comment.
This line should be added to installpsh-suse.sh too.
| exit 1 | ||
| fi | ||
|
|
||
| echo "Installing PowerShell to /opt/microsoft/powershell/$release in overwrite mode" |
There was a problem hiding this comment.
Maybe add this line to installpsh-suse.sh too?
There was a problem hiding this comment.
For this comment and the one below, they are about adding the ehco message to the same place in installpsh-suse.sh. Making this change won't need additional tests as it's just adding a log message. Could you please do it?
There was a problem hiding this comment.
This change is made, squashed and pushed - but this PR is now showing as "closed" ?
There was a problem hiding this comment.
@DarwinJS The timeline of this PR shows that you closed it ... But I see you already opened a new PR.
| DIST=`cat /etc/system-release |sed s/\ release.*//` | ||
| PSUEDONAME=`cat /etc/system-release | sed s/.*\(// | sed s/\)//` | ||
| REV=`cat /etc/system-release | sed s/.*release\ // | sed s/\ .*//` | ||
| if [[ $DIST == *"Amazon Linux"* ]] ; then |
There was a problem hiding this comment.
The code to check current OS and DistroBasedOn is different from other installsh- scripts targeting other Linux distros. Are you going to unify them all?
There was a problem hiding this comment.
Although it could be done, it's not absolutely necessary so I tacked toward minimal changes to anything I wasn't going to unit test before doing the PR. install-powershell.sh needs it to correctly call installpsh-amazonlinux.sh and then installpsh-amazonlinux.sh needs it to validate the correct installer. None of the others need the amazonlinux piece - but I can also understand keeping this bit of code the same. I don't currently have the bandwidth to test if I change the others.
Let me know what you think we should do with this and then I'll do one update and include the other two items you mentioned in this PR.
There was a problem hiding this comment.
Thanks for the clarification. I'm fine with the current change.
It would be more consistent if this piece of code can be unified, but that can be done in separate PRs. #closed
3 participants
as discussed in 5409: