Skip to content

[release/6.0] Add Rocky Linux RIDs (#74159) - #74164

Merged
carlossanlop merged 1 commit into
dotnet:release/6.0from
MarkpageBxl:release/6.0
Sep 8, 2022
Merged

[release/6.0] Add Rocky Linux RIDs (#74159)#74164
carlossanlop merged 1 commit into
dotnet:release/6.0from
MarkpageBxl:release/6.0

Conversation

@MarkpageBxl

@MarkpageBxlMarkpageBxl commented Aug 18, 2022

Copy link
Copy Markdown
Contributor

Fixes#74159
Add support for Rocky Linux RIDs on the 6.0 branch.

@ghostghost added area-Infrastructure-libraries community-contribution Indicates that the PR has been added by a community member labels Aug 18, 2022
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

Add support for Rocky Linux RIDs on the 6.0 branch.

Author:mhlindstr
Assignees:-
Labels:

area-Infrastructure-libraries

Milestone:-

@MarkpageBxl
MarkpageBxlforce-pushed the release/6.0 branch 2 times, most recently from 0f506eb to dd3bcddCompareAugust 18, 2022 18:41
@teo-tsirpanisteo-tsirpanis added this to the 6.0.x milestone Aug 18, 2022
@ViktorHofer

Copy link
Copy Markdown
Member

cc @carlossanlop

@carlossanlopcarlossanlop left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes seem to be roughly the same as in main, aside from the double brackets, and the package authoring changes that I don't understand.

@wfurt do you approve of this change as is?

Comment threadeng/native/init-distro-rid.sh Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems both single or double square brackets are allowed to evaluate expressions. I mention it because the main version only had single square brackets.

Source: http://mywiki.wooledge.org/BashFAQ/031

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

To be honest, I didn't even notice that single brackets where used on main, I only wished to add rocky as a valid alternative to the existing test. It doesn't have any functional impact here, as double brackets only give access to the extended Bash test syntax.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It seems double brackets are more correct. https://stackoverflow.com/a/2188369/8816314

If we have to do any corrections, we would have to add them to main, not here. But I think single brackets work too, if I'm understanding those two sources correctly.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Single brackets are compatible with /bin/sh (e.g. standard POSIX tests), whereas double brackets are a bash extension. I assume it has since been decided to not use bash extensions on main, but I would leave it as it is on servicing.

@carlossanlopcarlossanlop added the needs-author-action An issue or pull request that requires more info or actions from the author. label Sep 7, 2022
@ghostghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Sep 8, 2022

@carlossanlopcarlossanlop left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Package authoring suggestion addressed. LGTM.

@carlossanlop
carlossanlop merged commit 2cfbeab into dotnet:release/6.0Sep 8, 2022
@ghostghost locked as resolved and limited conversation to collaborators Oct 8, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-librariescommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@MarkpageBxl@ViktorHofer@carlossanlop@ericstj@smasher164@teo-tsirpanis@N3WWN