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 CentOS 8 to mangle-shebangs.sh #53

Closed
wants to merge 5 commits into from

Conversation

sjugge
Copy link

@sjugge sjugge commented Apr 15, 2020

update closed in favor of clean PR #55

Issue #, if available:

#52

Description of changes:

This PR extends the existing mangle-shebangs.sh to include CentOS 8. This will allow the package to be build and installed on CentOS 8.

Included in this PR:

  • mangle-shebangs.sh to include CentOS 8
  • Tests for CentOS 8
    - [x] README update to include Centos 8 as supported distro

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@sjugge sjugge mentioned this pull request Apr 15, 2020
@Cappuccinuo Cappuccinuo self-assigned this Apr 17, 2020
@Cappuccinuo Cappuccinuo self-requested a review April 17, 2020 15:03
README.md Outdated
@@ -132,7 +133,7 @@ or refer to the [documentation](https://docs.aws.amazon.com/efs/latest/ug/using-

## Upgrading stunnel for RHEL/CentOS

By default, when using the EFS mount helper with TLS, it enforces certificate hostname checking. The EFS mount helper uses the `stunnel` program for its TLS functionality. Please note that some versions of Linux do not include a version of `stunnel` that supports TLS features by default. When using such a Linux version, mounting an EFS file system using TLS will fail.
By default, when using the EFS mount helper with TLS, it enforces certificate hostname checking. The EFS mount helper uses the `stunnel` program for its TLS functionality. Please note that some versions of Linux do not include a version of `stunnel` that supports TLS features by default. When using such a Linux version, mounting an EFS file system using TLS will fail.
Copy link
Contributor

Choose a reason for hiding this comment

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

The tail blank space is not needed.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry about that, Git config stripped whitespace, reverted in 6fa35aa

README.md Outdated
@@ -11,6 +11,7 @@ The `efs-utils` package has been verified against the following Linux distributi
| Amazon Linux 2017.09 | `rpm` | `upstart` |
| Amazon Linux 2 | `rpm` | `systemd` |
| CentOS 7 | `rpm` | `systemd` |
| CentOS 8 | `rpm` | `systemd` |
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not change this file currently, we have a full process of qualifying linux distributions.

Copy link
Author

Choose a reason for hiding this comment

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

README reverted in 6fa35aa

@@ -3,9 +3,10 @@
SYSTEM_RELEASE_PATH=/etc/system-release
RHEL8_REGEX="Red Hat Enterprise Linux release 8"
FEDORA_REGEX="Fedora release"
CENTOS8_REGEX="CentOS Linux release 8"

# RHEL8 and Fedora30+ both treat shebangs of the form "#!/usr/bin/env python" as errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also modify the comment?

RHEL8, Fedora30+ and CentOS8 both treat shebangs of the form "#!/usr/bin/env python" as errors

Copy link
Author

Choose a reason for hiding this comment

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

Comment updated

Copy link
Contributor

@Cappuccinuo Cappuccinuo left a comment

Choose a reason for hiding this comment

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

LGTM. Please don't change README.md for now, and add CentOS8 to comment in mangle-shebangs.sh.

Also, please merge all the commits to one commit, thanks.

@sjugge sjugge mentioned this pull request Apr 17, 2020
@sjugge
Copy link
Author

sjugge commented Apr 17, 2020

@Cappuccinuo

please merge all the commits to one commit

I couldn't update the branch for this PR without messing with the history, clean single-commit PR opened -> #55

@sjugge sjugge closed this Apr 17, 2020
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.

None yet

2 participants