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

UCRT warnings on win10 #3736

Merged
merged 2 commits into from Feb 28, 2019
Merged

UCRT warnings on win10 #3736

merged 2 commits into from Feb 28, 2019

Conversation

cowo78
Copy link
Contributor

@cowo78 cowo78 commented Sep 11, 2018

bindepend: don't generate warnings regarding missing UCRT dependencies on windows 10 as these files do not exist on purpose (see issue #1566)

@htgoebel
Copy link
Member

Thank you for you pull-request. Please move the first commit into a separate pull-request since it is unrelated, much more complex and need separate review. Hint: is_win_10 is defined in your first commit.

Please reword your commit messages to comply to our Commit Message Rules. You also need to submit a changelog entry so our users can learn about your change. (This is new since a few weeks now.)

Also you should base your commit on the latest development HEAD. You are currently basing on a commit which is more then one year old.

When updating a pull-request, you can simply (force) push the updated branch to github again. This will automatically update the pull-request (which follows the branch, not the commit). So you do not need to close the pull-request and open a new one. This also has the benefit that the discussion history is kept. For detailed instructions please read Updating a Pull-Request in the manual.

Thanks.

@htgoebel htgoebel added state:needs more work This PR needs more work area:Unicode labels Sep 11, 2018
@htgoebel
Copy link
Member

Oh, I just see you already have two pull-requests. Please base the second on the develop head, too.

@htgoebel
Copy link
Member

Something went wrong with this pull-request. Now it does not contains anything related UCRT to.

@cowo78
Copy link
Contributor Author

cowo78 commented Dec 10, 2018

I rebased on current develop and force-pushed to ucrt_win10 branch.
Don't know what was wrong with plain push but once rebased the local and remote branches appeared to be diverging.

@htgoebel
Copy link
Member

How did you rebase? This should work:

git checkout develop
git pull origin
git checkout ucrt_win10
git rebase develop

@cowo78
Copy link
Contributor Author

cowo78 commented Dec 13, 2018

How did you rebase? This should work:

git checkout develop
git pull origin
git checkout ucrt_win10
git rebase develop

Common process, yeah. I would say I did exactly these steps but evidently I must have done something else along the way.
Anyway the PR should be OK right now, I just had to force-push instead of simple push.

@htgoebel htgoebel force-pushed the ucrt_win10 branch 2 times, most recently from 4af90f4 to fa82207 Compare February 26, 2019 16:00
Giuseppe Corbelli added 2 commits February 27, 2019 13:15
These files do not exist on Windows 10 on purpose (see issue pyinstaller#1566).

[skip travis] no need to test on non-windows platforms
@htgoebel htgoebel merged commit 6c9fa53 into pyinstaller:develop Feb 28, 2019
@htgoebel
Copy link
Member

Thanks for the pull-request. I merged it just a minute ago.

@cowo78 cowo78 deleted the ucrt_win10 branch June 21, 2019 08:29
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
state:needs more work This PR needs more work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants