[go: nahoru, domu]

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

Avoid letting crouton run inside crostini #3862

Merged
merged 3 commits into from
Oct 1, 2018
Merged

Avoid letting crouton run inside crostini #3862

merged 3 commits into from
Oct 1, 2018

Conversation

dnschneid
Copy link
Owner

Reviewers: please also comment on the README and error message wording.

@DennisLfromGA
Copy link
Collaborator

LGTM 👍 ( not that it matters of course )
And sound great to me too. 😂

@picostove
Copy link
Collaborator

LGTM

if ! awk -F= '/_RELEASE_BUILD_NUMBER=/ { exit int($2) < '"${CROS_MIN_VERS:-0}"' }' \
if [ -n "$DOWNLOADONLY" ]; then
:
elif ! grep -q '_RELEASE_BUILD_NUMBER=' /etc/lsb-release 2>/dev/null; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, is this enough or should it be even more qualified with 'CHROMEOS_RELEASE_BUILD_NUMBER' here and on line 145?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I think that would mess up Chromium OS

Copy link
Collaborator
@DennisLfromGA DennisLfromGA Aug 8, 2018

Choose a reason for hiding this comment

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

Well, I thought so too at first but after checking it seems all the Chromium OS derivatives use the 'CHROMEOS_' prefix. Here's the lsb-release files for CrOS, Chromium OS, Cloudready, & Flint OS (yes, I have all of them)

But I guess things can change so whatever you decide is fine with me.

@dnschneid dnschneid merged commit d982ebf into master Oct 1, 2018
@dnschneid dnschneid deleted the versioncheck branch December 6, 2018 23:50
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.

3 participants