8000 osx : fix the inverted version test by pmalhaire · Pull Request #772 · hishamhm/htop · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Sep 8, 2020. It is now read-only.

osx : fix the inverted version test - #772

Merged
hishamhm merged 1 commit into
hishamhm:masterfrom
pmalhaire:macos_keep_not_high_sierra_ok_fixed
Apr 5, 2018
Merged

osx : fix the inverted version test#772
hishamhm merged 1 commit into
hishamhm:masterfrom
pmalhaire:macos_keep_not_high_sierra_ok_fixed

Conversation

@pmalhaire
@pmalhaire pmalhaire commented Mar 29, 2018
Copy link
Copy Markdown
Contributor

switched flags for this #728

@pmalhaire pmalhaire changed the title fix the switched versions osx : fix the inverted version test Mar 29, 2018
@Explorer09
Explorer09 commented Mar 30, 2018 •
edited
Loading
Copy link
Copy Markdown
Contributor

Hello. I wonder what's the reason for inverting the function behavior of CompareKernelVersion() ?
Also, when we are improving the function, is it okay to add static qualifier to CompareKernelVersion? If you are familiar with C you know what it means.

@pmalhaire
pmalhaire commented Mar 30, 2018 •
edited
Loading
Copy link
Copy Markdown
Contributor Author

@Explorer09 I do invert the function because I made a mistake when l wrote it. See #728
It makes sense to make it static.

@Smattr Smattr mentioned this pull request Mar 31, 2018
Closed
@pmalhaire
pmalhaire commented Mar 31, 2018 •
edited
Loading
Copy link
Copy Markdown
Contributor Author

@hishamhm I accidentaly inverted the test when I compacted the code. This is a fix for this

@ylluminarious ylluminarious mentioned this pull request Mar 31, 2018
Merged
@ylluminarious
ylluminarious commented Apr 2, 2018
Copy link
Copy Markdown

@pmalhaire @Explorer09 After testing this patch on my own and verifying the test with some logging, I can confirm that this patch fixes the problem and it should no longer occur. Unless, of course, something other than DarwinProcess_scanThreads is causing the problem, but there is no evidence to indicate that.

@ylluminarious
ylluminarious commented Apr 4, 2018
Copy link
Copy Markdown

I applied this patch and left the resultant program running for 4 hours on 10.13.3 and didn't crash... I suggest that this patch be merged as soon as possible.

@hishamhm
hishamhm merged commit 0505a7c into hishamhm:master Apr 5, 2018
@hishamhm 8000
hishamhm commented Apr 5, 2018
Copy link
Copy Markdown
Owner

Merged! Thanks, @pmalhaire! A new release is coming this weekend!

@ylluminarious
ylluminarious commented Apr 5, 2018
Copy link
Copy Markdown

@hishamhm Thanks again! Sorry for the false relief in the previous PR... but thanks for your patience!

@pmalhaire
pmalhaire commented Apr 6, 2018 •
edited
Loading
Copy link
Copy Markdown
Contributor Author

@hishamhm thanks to you and sorry for the mess I made by inverting my test.
see you at FOSDEM

hishamhm pushed a commit that referenced this pull request Apr 6, 2018
@pmalhaire @hishamhm
macOS: fix the switched version test (#772)
7a71f9c
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@pmalhaire @Explorer09 @ylluminarious @hishamhm
0