Skip to content

build: fix and document the symlinks scripts for Windows#10213

Closed
marclaval wants to merge 1 commit into
angular:masterfrom
marclaval:fixWindowsAfterRC2
Closed

build: fix and document the symlinks scripts for Windows#10213
marclaval wants to merge 1 commit into
angular:masterfrom
marclaval:fixWindowsAfterRC2

Conversation

@marclaval

Copy link
Copy Markdown
Contributor

Some more fixes, some commands are still broken and will require more work:

./test.sh tools
./test.sh node

@vicb

vicb commented Jul 21, 2016

Copy link
Copy Markdown
Contributor

@Mlaval it's not clear to me what this fixes and if the PR is a WIP or ready to go ?

@fknop

fknop commented Jul 21, 2016

Copy link
Copy Markdown
Contributor

@vicb I know that when I made a PR, the create / remove symlinks did not work due to the \r at the end of the package name. Looks like the ::-1 operator removes the last character but I could not make it work on my Git Bash.

@Mlaval What terminal are you using ?
I did some research, looks like it's only available for latest versions of bash. I still had an older version on this computer. http://unix.stackexchange.com/a/144330

@vicb vicb added state: blocked action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 21, 2016
@marclaval

Copy link
Copy Markdown
Contributor Author

I'm using a recent version of Git Bash (2.6.2) and this PR fixes the issue you described with the '\r'.
Sorry for not explaining when I opened the PR.

@vicb: there are more things to fix on the platform, but this one allows to at least run tests in a browser.

@vicb

vicb commented Jul 22, 2016

Copy link
Copy Markdown
Contributor

I did some research, looks like it's only available for latest versions of bash.

Is it ok to merge still ?

@marclaval marclaval force-pushed the fixWindowsAfterRC2 branch from eed5622 to 2d7f7a8 Compare July 25, 2016 08:18
@marclaval marclaval changed the title build: fix some issues on Windows platforms build: fix and document the symlinks scripts for Windows Jul 25, 2016
@marclaval

Copy link
Copy Markdown
Contributor Author

I've updated the PR with the findings of @fknop so that the scripts also work in older bash.

The commit message is now more explicit.

It should be ok to merge.

@vicb

vicb commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

When it's been :)

@vicb

vicb commented Jul 25, 2016

Copy link
Copy Markdown
Contributor

Green... Stupid phone!

@marclaval

Copy link
Copy Markdown
Contributor Author

It is now, the e2e job went flaky.

@vicb

vicb commented Jul 26, 2016

Copy link
Copy Markdown
Contributor

Lgtm. Can not merge from the mobile UI. Feel free to merge

@marclaval marclaval closed this in ca16fc2 Jul 26, 2016
@marclaval marclaval deleted the fixWindowsAfterRC2 branch December 11, 2017 10:01
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: review The PR is still awaiting reviews from at least one requested reviewer cla: yes state: blocked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants