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.

Sign up for GitHub

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

Jump to bottom

feat:README.md: Section "Install ble-testsuite" is no longer valid #35

Merged
merged 3 commits into from
Oct 7, 2023
Merged

feat:README.md: Section "Install ble-testsuite" is no longer valid #35

merged 3 commits into from
Oct 7, 2023

Conversation

Chetax
Copy link
Contributor

@Chetax Chetax commented Oct 4, 2023

Fixes Issue #30

📑 Description

Updated Execution command in section "Install ble-testsuite"

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed

@Chetax
realted to issue 30 & Update README.md
126f91a 
README.md: Section "Install ble-testsuite" is no longer valid
@netlify
Copy link

netlify bot commented Oct 4, 2023 •
edited
Loading

✅ Deploy Preview for ble-testsuite canceled.

Name Link
🔨 Latest commit e50ca55
🔍 Latest deploy log https://app.netlify.com/sites/ble-testsuite/deploys/65210cb1963fb50008ed72cd

gmacario
gmacario requested changes Oct 4, 2023
View reviewed changes
Copy link
Member

@gmacario gmacario left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Many thanks for your contribution @Chetax!

I believe there are too many shell invocations.
The outer /bin/bash -c is no longer needed once you add a | sh after curl

@gmacario gmacario added bug Something isn't working documentation Improvements or additions to documentation labels Oct 4, 2023
@gmacario gmacario added this to the dev-cw40 milestone Oct 4, 2023
gmacario
gmacario requested changes Oct 5, 2023
View reviewed changes
Copy link
Member

@gmacario gmacario left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please try the command. I believe you should also remove $( and ) to make it work

@gmacario gmacario self-requested a review October 7, 2023 07:46
gmacario
gmacario approved these changes Oct 7, 2023
View reviewed changes
Copy link
Member

@gmacario gmacario left a comment

Choose a reason for hiding this comment

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

With the latest commit LGTM.

@gmacario gmacario merged commit 4ff4de6 into B-AROL-O:main Oct 7, 2023
5 checks passed
@gmacario gmacario mentioned this pull request Oct 11, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmacario gmacario gmacario approved these changes

Assignees

@Chetax Chetax

Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Milestone
2023-cw40
Development

Successfully merging this pull request may close these issues.
2 participants
@Chetax @gmacario