Skip to content

Fix spaces in parent folder name#18

Closed
protik09 wants to merge 3 commits into
raspberrypi:masterfrom
protik09:master
Closed

Fix spaces in parent folder name#18
protik09 wants to merge 3 commits into
raspberrypi:masterfrom
protik09:master

Conversation

@protik09

Copy link
Copy Markdown

This commit fixes Issue #17

@aallan

aallan commented Mar 15, 2021

Copy link
Copy Markdown

Not needed on Raspberry Pi OS, but might be worthwhile merging in any case?

@aallan aallan requested a review from liamfraser March 15, 2021 10:32
@JamesH65

Copy link
Copy Markdown

Why not needed on PiOS? If there is space in the installation folder name, I think pico-setup would fail?

Comment thread pico_setup.sh

# Number of cores when running make
JNUM=4
JNUM=$(cat /proc/cpuinfo | grep processor | wc -l)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please make this change in a separate PR, as it's unrelated the the spaces-in-path issue.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ahh.. Apologies I missed that. Will do.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like you removed this, but then added it back again??
You might find things easier to manage if you do them on separate branches, rather than doing everything on protik09:master 🙂

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ping @protik09 again. Can you please update this PR to remove this change?

@aallan

aallan commented Mar 15, 2021

Copy link
Copy Markdown

Why not needed on PiOS? If there is space in the installation folder name, I think pico-setup would fail?

Doh! Fair point. I was totally in the mindset of "well it works on my machine!" Blame it on Monday and not enough coffee? 🤦‍♂️

Comment thread pico_setup.sh
do
echo "Building $e"
cd $e
cd "$e"

@lurch lurch Mar 15, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Technically not needed here, as neither blink nor hello_world contain spaces 😉 (but I guess it doesn't hurt)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd be inclined to keep the quotes everywhere they may be needed, to make it future proof/consistent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed, that's why I said "(but I guess it doesn't hurt)"

Temporary fix to remove unrelated info from pull request for Issue raspberrypi#17
This gives max compile speed on any Raspberry Pi
@aallan

aallan commented Apr 6, 2021

Copy link
Copy Markdown

Superseded by #20. But note that one of openocd's submodules doesn't handle spaces and there doesn't seem to be a way around that?

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.

4 participants