Skip to content

CMake: added missing HAVE_BUILTIN_AVAILABLE, HAVE_CLOCK_GETTIME_MONOTONIC#3097

Closed
dmitrykos wants to merge 1 commit intocurl:masterfrom
dmitrykos:fix_missing_CMake_defines
Closed

CMake: added missing HAVE_BUILTIN_AVAILABLE, HAVE_CLOCK_GETTIME_MONOTONIC#3097
dmitrykos wants to merge 1 commit intocurl:masterfrom
dmitrykos:fix_missing_CMake_defines

Conversation

@dmitrykos
Copy link
Copy Markdown
Contributor

Added missing HAVE_BUILTIN_AVAILABLE (define and test), HAVE_CLOCK_GETTIME_MONOTONIC (test). Tested with CMake on Apple OSX and Android NDK (r17b, Windows).

…missing test for HAVE_CLOCK_GETTIME_MONOTONIC
@dmitrykos
Copy link
Copy Markdown
Contributor Author

@snikulov, here is new pull request.

@snikulov snikulov added the cmake label Oct 5, 2018
@snikulov snikulov self-requested a review October 5, 2018 07:23
@snikulov
Copy link
Copy Markdown
Contributor

snikulov commented Oct 5, 2018

@bagder Daniel, can I merge this one?

@bagder
Copy link
Copy Markdown
Member

bagder commented Oct 5, 2018

Please do, but also please amend the commit message as it doesn't really follow our style - and a "closes #3097" would be great then as well. (Ie: don't use the github merge button.)

@snikulov snikulov closed this in 667b572 Oct 5, 2018
@snikulov
Copy link
Copy Markdown
Contributor

snikulov commented Oct 5, 2018

@bagder landed on master.
I use --no-ff as was suggested in command lines on GitHub and now we have the merge commit in the tree.
If it's not good please remove it with force push.

@bagder
Copy link
Copy Markdown
Member

bagder commented Oct 5, 2018

We don't do merge commits, please rebase it properly next time.

And we don't force-push on master. Ever.

@MarcelRaad
Copy link
Copy Markdown
Member

@snikulov Do you know https://github.com/curl/curl/wiki/push-access-guidelines#how-to-work-with-a-pr-branch? That's what I'm trying to do, except that I'm doing the (You could instead use git rebase -i and choose those commits to squash or reword.)

@snikulov
Copy link
Copy Markdown
Contributor

snikulov commented Oct 5, 2018

@MarcelRaad Thanks for the tip.
I've used command line help from github. The only thing which is wrong was adding --no-ff
Rebase will not show merge commit (AFAIR) because it is the same commit except special attribute.

@snikulov
Copy link
Copy Markdown
Contributor

snikulov commented Oct 5, 2018

image

This one should be --ff-only by default

@bagder
Copy link
Copy Markdown
Member

bagder commented Oct 5, 2018

(Those instructions are hard-coded by github itself, we can't change them.)

That would just make the merge fail very often. Just do it like this instead:

git checkout master
git merge [name of branch]
git rebase origin/master

... verify with git log that everything looks fine, then...

git push origin master

If there are multiple commits and you'd rather squash a few (I personally like to just 'f'ixup them), just do -i on the rebase line:

git rebase -i origin/master

@lock lock bot locked as resolved and limited conversation to collaborators Jan 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

4 participants