-
Notifications
You must be signed in to change notification settings - Fork 341
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
Rework the ReadMe to be more quick-start friendly #1173
Conversation
|
||
If you are building on Windows you need to generate `NMake Makefiles`, you should run `cmake .. -G "NMake Makefiles"` | ||
_Mac and Linux_ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would suggest splitting this into 2 subsections:
- Build from scratch (build with dependencies=ON)
- Build using system libraries (build with dependencies=OFF). For this, you would also need to install the necessary dependencies using system package manager
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to add this to the PR description as a future addition to consider, but I'm not set on doing this quite yet since for the smoothest onboarding experience we would recommend to do build dependencies to ensure a supported version of a dependency library is being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To add to that, it's not as simple as just including the install instructions with the correct versions: users might already have some of the dependency libraries installed with unsupported versions and there would be potential for CMake or pkgconfig to link the incorrect ones.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1173 +/- ##
===========================================
- Coverage 16.48% 16.30% -0.18%
===========================================
Files 50 50
Lines 7019 7095 +76
===========================================
Hits 1157 1157
- Misses 5862 5938 +76 ☔ View full report in Codecov by Sentry. |
…n the Quick Start section
README.md
Outdated
_Windows_ | ||
```bat | ||
choco install gstreamer --version=1.22.8 | ||
choco install gstreamer-devel --version=1.22.8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How come the gstreamer version is only specified in the windows? Does mac and linux requires certain gstreamer versions? Is this the only supported gstreamer version for windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely not the only supported version, but for the windows build, we have relied on using the pkg-config library bundled with the gstreamer installation. We've found that some versions of gstreamer do not have the pkg-config library. I believe we may be able to switch to just using pkg-config-lite, installed via choco, then we can remove this version specifier.
* Start remodeling the ReadMe * Fix typo, add italics to OS names * add todo * Finish up the body * Update ToC * Move stuff, fix typos * Improve title * Add a line * Add link for cmake options * Fix ToC * Pi->Linux * Add a comma * Address feedback * Add more Windows instructions * Restructure Debugging section, add more links * Add some periods * Add link to Java SDK * Add link to Producer C SDK in Key Features section * Convert cmake options list to table * Add kvssink parameters table, modify credential-path description, cleanup cmake table * Add stream from file kvssink sample instructions * Add instructions for how/where to view the footage from the samples in the Quick Start section * Address comments * Spelling and grammar * Remove accidental link * Downgrade Mac CI GCC version 13->12 * Add AWS Authentication to quickstart section * megabytes -> mebibytes * Address comments * Fix paths * Format element names * Remove code formatting from links * Format fixing * Update README.md * Update README.md
* Rework the ReadMe to be more quick-start friendly (#1173) * Start remodeling the ReadMe * Fix typo, add italics to OS names * add todo * Finish up the body * Update ToC * Move stuff, fix typos * Improve title * Add a line * Add link for cmake options * Fix ToC * Pi->Linux * Add a comma * Address feedback * Add more Windows instructions * Restructure Debugging section, add more links * Add some periods * Add link to Java SDK * Add link to Producer C SDK in Key Features section * Convert cmake options list to table * Add kvssink parameters table, modify credential-path description, cleanup cmake table * Add stream from file kvssink sample instructions * Add instructions for how/where to view the footage from the samples in the Quick Start section * Address comments * Spelling and grammar * Remove accidental link * Downgrade Mac CI GCC version 13->12 * Add AWS Authentication to quickstart section * megabytes -> mebibytes * Address comments * Fix paths * Format element names * Remove code formatting from links * Format fixing * Update README.md * Update README.md * Address comments * Address comments * Add link to AWS KVS Raspberry Pi doc
What was changed?
The ReadMe was re-worked to be more quick-start friendly. Missing information was added and made into tables. Typos and grammar issues were addressed.
(More details in the Overview section below.)
Why was it changed?
To improve the ReadMe and correct outstanding issues.
How was it changed?
By modifying the ReadMe document to address the above issues.
What testing was done for the changes?
The document was pasted into a spelling and grammar checker.
Overview
Future Improvements
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.