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 README.md: Create ConfigOverride.xcconfig by command line #180

Merged
merged 5 commits into from
May 16, 2024

Conversation

bjornoleh
Copy link
Contributor

No description provided.

@marionbarker
Copy link
Contributor

I recommend adding the BuildTrio.sh instructions below the line that says there is a script available.

Copy the code embedded here into the Trio README:

https://github.com/loopandlearn/lnl-scripts/tree/main?tab=readme-ov-file#scripts-for-the-trio-app

Code inspection OK of what you added looks good but I can’t test it right now.

README.md Outdated Show resolved Hide resolved
@MikePlante1 MikePlante1 self-requested a review May 14, 2024 17:49
MikePlante1
MikePlante1 previously approved these changes May 14, 2024
Copy link
Contributor

@MikePlante1 MikePlante1 left a comment

Choose a reason for hiding this comment

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

Did you want to add the build script that Marion mentioned? If you’d prefer that left for a different PR, then LGTM.

@bjornoleh
Copy link
Contributor Author

We can fit that in here if someone has a chance to push it now, otherwise merge as is.

@MikePlante1
Copy link
Contributor

We can fit that in here if someone has a chance to push it now, otherwise merge as is.

Thoughts on this?

CLI readme

@bjornoleh
Copy link
Contributor Author

We can fit that in here if someone has a chance to push it now, otherwise merge as is.

Thoughts on this?

Looks great!

@marionbarker
Copy link
Contributor

I reviewed the entire README file after the last commit/
Please add the missing links that are now known.

Look for TODO:

  • There are 3 of them
  • I know at least one of those is now known.

Tests:

  • I tested the addition of the script and it worked.
  • I tested the manual instructions and they work.
  • Tests performed using a test phone running iOS 17.4.1 with Xcode 15.3

Docs, FAX, Oref0
@MikePlante1
Copy link
Contributor

I reviewed the entire README file after the last commit/ Please add the missing links that are now known.

Look for TODO:

  • There are 3 of them
  • I know at least one of those is now known.

I added a link for Docs (and links to FAX and Oref0), but should probably wait to add the website (diy-trio.org) and the Crowdin (until @t1dude says it's ready for contributions)

marionbarker
marionbarker previously approved these changes May 14, 2024
Copy link
Contributor

@marionbarker marionbarker left a comment

Choose a reason for hiding this comment

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

LGTM

@MikePlante1 MikePlante1 self-requested a review May 14, 2024 20:16
MikePlante1
MikePlante1 previously approved these changes May 14, 2024
Copy link
Contributor

@LiroyvH LiroyvH left a comment

Choose a reason for hiding this comment

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

Note: when you list it like so (as it is in the current proposal):

git clone --branch=<branch> --recurse-submodules https://github.com/nightscout/Trio.git
cd Trio

Then if the default shell is still set to bash, copy/pasting it to Terminal will automatically execute the first command; which will fail because of the <branch>. This will thus result in an error such as:
"bash: branch: No such file or directory"

I'd suggest writing it as follows, so that when copy/pasting directly to Terminal the user will - regardless of which shell they're using - always have an opportunity to change the <branch> portion before execution (and it auto-terminates if the git command fails for whatever reason; preventing an error for cd on such an occasion):

git clone --branch=<branch> --recurse-submodules https://github.com/nightscout/Trio.git && cd Trio

@MikePlante1
Copy link
Contributor

git clone --branch=<branch> --recurse-submodules https://github.com/nightscout/Trio.git && cd Trio

Thanks for the suggestion.

2-line in zsh: success
2-line in bash: fail
1-line in zsh: success
1-line in bash: success

I'll commit that now with you as the author

both single and double line versions worked in zsh, but only the single line version worked in bash
Copy link

@becksen becksen left a comment

Choose a reason for hiding this comment

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

reviewed

@MikePlante1 MikePlante1 merged commit 56a3f87 into dev May 16, 2024
@MikePlante1 MikePlante1 deleted the create_ConfigOverride_CLI branch May 16, 2024 03:35
@MikePlante1 MikePlante1 mentioned this pull request May 16, 2024
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.

5 participants