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

Update Windows instructions in README.md #4184

Closed
wants to merge 4 commits into from
Closed

Update Windows instructions in README.md #4184

wants to merge 4 commits into from

Conversation

thejohnjansen
Copy link
Contributor

  • Update readme for windows

I had a tough time getting this working on windows. Updated readme to
help.

thejohnjansen and others added 2 commits November 8, 2016 14:27
* Update readme for windows

I had a tough time getting this working on windows. Updated readme to
help.
Microsoft Edge's current set of tests for an ECMAScript6 Modules
implementation via the script element (<script type="module">).
@halindrome
Copy link
Contributor

LGTM!

Copy link
Contributor

@sideshowbarker sideshowbarker left a comment

Choose a reason for hiding this comment

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

Thanks—glad we’re getting more help for people to get their wpt environment set up on Windows successfully. See other comments for minor nits.

Leave the default install settings so Python will be installed to the c:\python2x directory.

Be sure to add that directory to your `%Path%`
[Environment Variable](http://www.computerhope.com/issues/ch000549.htm).
Copy link
Contributor

Choose a reason for hiding this comment

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

The %Path% syntax is Windows-specific, right? And the http://www.computerhope.com/issues/ch000549.htm info is also Windows-specific. So it seems like this should be moved to that Windows Notes section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for that. I've moved that sentence to the Windows Notes section now.

Moving the PATH information to Windows notes section.
@sideshowbarker sideshowbarker changed the title Johnjanreadme (#17) Update Windows instructions in README.md Nov 25, 2016
@sideshowbarker
Copy link
Contributor

@thejohnjansen seems the “Add ECMAScript 6 Modules test cases” change (e6fdc7d / 360d873) got inadvertently committed to the branch. I guess that happened because the PR was made from the master branch of https://github.com/MicrosoftEdge/web-platform-tests.

To prevent things like this, it’s always better to create a branch in your wpt fork for each PR and commit there instead of master, and then open the PR from that non-master branch.

If you want, I can fix this so we can merge this PR, but it means I would need to force-push to the https://github.com/MicrosoftEdge/web-platform-tests master branch to cause the “Add ECMAScript 6 Modules test cases” change (https://github.com/MicrosoftEdge/web-platform-tests/commit/e6fdc7d91a288d29ca07257ff74b87acb97a3928) to be (temporarily) dropped from it.

After I did that and merge this PR you’d need to pull from https://github.com/MicrosoftEdge/web-platform-tests and then re-merge that “ES6 Modules test cases” change to master there again.

If you prefer to do it yourself (rather me being the one doing the force-push), what you’d need to is:

  1. git rebase -i 2782308f4187e73043324272552cf1bd90edc784
    Your text editor will open and show you something like this: untitled

  2. Change the above in your text editor so it looks like this: untitled

    That is, change the pick e6fdc7d Add ECMAScript 6 Modules test cases line to:
    d e6fdc7d Add ECMAScript 6 Modules test cases
    …and change the pick 50a9973 Code review feedback line to:
    s 50a9973 Code review feedback

  3. Save and close your editor.
    Your editor will re-open. Just save what it shows as-is and close.

  4. git push -f

After that we merge this PR but you’d still need to pull from https://github.com/MicrosoftEdge/web-platform-tests and then re-merge that “ES6 Modules test cases” change to master there again.

@thejohnjansen
Copy link
Contributor Author

hey @sideshowbarker, thanks for the detailed summary. I tried to do the commands myself, but I got an error when I ran git push -f
Successfully rebased and updated refs/heads/master.
C:\Users\johnjan\Documents\GitHub\web-platform-tests [master ↕]> git push -f
Counting objects: 3, done.
Delta compression using up to 12 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 1023 bytes | 0 bytes/s, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: Resolving deltas: 100% (2/2), completed with 2 local objects.
remote: error: GH006: Protected branch update failed for refs/heads/master.
remote: error: Cannot force-push to a protected branch
To https://github.com/MicrosoftEdge/web-platform-tests.git
! [remote rejected] master -> master (protected branch hook declined)
error: failed to push some refs to 'https://github.com/MicrosoftEdge/web-platform-tests.git'
C:\Users\johnjan\Documents\GitHub\web-platform-tests [master ↕]>

@thejohnjansen
Copy link
Contributor Author

PS: I cannot believe I didn't create a new branch and then complicated this PR like that. Embarassing.

@sideshowbarker
Copy link
Contributor

@thejohnjansen no worries—I just went ahead and merged it on the command from my local copy of the branch

@gsnedders
Copy link
Member

This is part of #3776. (Ignore me. I'm doing housekeeping.)

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

5 participants