Added version-check to CI #543
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adding script permissions
|
Thanks for PR! Will review it in upcoming days. |
adrianopol
reviewed
Aug 31, 2023
| exit 1 | ||
| else | ||
| echo "version check OK" | ||
| fi |
There was a problem hiding this comment.
No newline at end of file
:)
Comment on lines
11
to
16
| echo | ||
| echo "version check FAILED" | ||
| echo "the git tag name and version declared in version.h is not the same" | ||
| echo "${tagname} git version tag" | ||
| echo "${header_tagname} header version tag" | ||
| echo |
There was a problem hiding this comment.
echo >&2 is preferred here, I assume
There was a problem hiding this comment.
Hey so in the other CI check scripts, such as check-formatting , echo is used, even for error messages, should I stick with that or make the change to echo >&2 ?
There was a problem hiding this comment.
@dheerajd5 , hello, this behavior is preferred, yes, as soon as it doesn't break the script's functionality.
|
|
||
| tagname=$(git tag -l) | ||
| tagname=${tagname: -5} | ||
| header_tagname=$(python3 scripts/scons_helpers/parse-version.py) |
There was a problem hiding this comment.
more secure way:
Suggested change
| header_tagname=$(python3 scripts/scons_helpers/parse-version.py) | |
| header_tagname="$(python3 scripts/scons_helpers/parse-version.py)" |
| header_tagname=$(python3 scripts/scons_helpers/parse-version.py) | ||
|
|
||
| if [[ "$tagname" != "$header_tagname" ]]; then | ||
| echo |
There was a problem hiding this comment.
nit: common indentation size in shell scripts is 2 spaces
| echo "${header_tagname} header version tag" | ||
| echo | ||
| exit 1 | ||
| else |
There was a problem hiding this comment.
BTW, else is useless here (as soon as you do exit in the first branch)
| echo | ||
| exit 1 | ||
| else | ||
| echo "version check OK" |
There was a problem hiding this comment.
nit:
Suggested change
| echo "version check OK" | |
| echo "version check SUCCEEDED" |
Co-authored-by: Andrew <4029800+adrianopol@users.noreply.github.com>
- rename roc_frame_encoding to roc_format (format is lower level setting compared to encoding; encoding defines format, channels, and rate) - remove remove packet_sample_rate and packet_channels (this parameters are derived from packet encoding)
- replace two fields max_latency_overrun and max_latency_underrun with a single field latency_tolerance - rename broken_playback_timeout to choppy_playback_timeout - remove breakage_detection_window (derive reasonable value from choppy_playback_timeout)
- add ChannelLayout, which defines meaning of channel numbers (mono, stereo, surround, multitrack) - add ChannelSet, which replaces channel_mask_t, and supports larger number of channels (up to 256 instead of 32) - add ChannelPosition, that identifies channel numbers for surround sound (currently only L and R) - use ChannelSet instead of channel_mask_t everywhere, including SampleSpec and ChannelMapper - upgrade ChannelMapper to properly map between different layouts (mono, stereo, multitrack) Surround sound is not fully supported yet: only L and R channels are currently available in surround layout.
- make SlabPool template class - add template parameter EmbeddedCapacity - extract implementation to non-template class SlabPoolImpl to avoid code size growth
- rename FormatMap::format() to FormatMap::find_by_pt() - add FormatMap::find_by_spec() - add FormatMap::add_format() - use SlabPool instead of Array to allocate formats (this allows safely returning pointers to formats to callers) - use Hashmap to search formats (for speedup) - make FormatMap thread-safe - move FormatMap from Sender/Receiver to Context These preparations will allow to implement C API for registering custom formats (e.g. with sample rate not covered by built-in formats).
- rename roc_channel_set to roc_channel_layout (because now it will define not only list of enabled channels, but also meaning of those channels, i.e. mono vs surround vs multitrack) - support two layouts: ROC_CHANNEL_LAYOUT_MONO ROC_CHANNEL_LAYOUT_STEREO - support two packet encodings: ROC_PACKET_ENCODING_AVP_L16_MONO ROC_PACKET_ENCODING_AVP_L16_STEREO - if packet_encoding is not set, automatically choose packet encoding that matches frame encoding
(for consistency with recent renames in C API)
- add roc_media_encoding struct which defines rate, channels, and format - use it in roc_sender_config and roc_receiver_config instead of multiple fields
Add new method roc_context_register_encoding(), that allows to register custom encodings on sender and receiver. It is useful when user needs to force custom packet encodings, but doesn't use signaling protocols (like RTSP), and instead wants to use static encodings table or manage signalling manually.
- change load factor from Java to Go variant - allow constructing hashmap without arena - do not implicitly grow hashmap in constructor - ensure that number of elements defined by EmbeddedCapacity can be always allocated without arena - add corresponding tests
Add to new function roc_sender_unlink() and roc_receiver_unlink() which allow to disconnect and remove previously created slots.
- if error happens during configure, bind, or connect, the whole slot is disabled and marked broken - broken slot is excluded from pipeline, and its sockets are closed - broken port still keeps slot number reserved - user should unlink broken slots to free remaining resources and allow slot index reuse - new logic is covered with tests - detailed tests for error handling are moved from public_api level to roc_peer level because there we have more information
- rename roc_peer module to roc_node, because we're going to add pipelines that are not network peers - use term "node" instead of "peer" everywhere
Co-authored-by: Andrew <4029800+adrianopol@users.noreply.github.com> Code Review changes Clean up commit
|
Hey, I rebased over existing commits by mistake, I'll put up a new PR with the code reviewed changes. |
|
Hi, there is no need to close this one, you may just fix your branch locally (e.g. rebase on |
|
Oh, sorry I'm new to this, I'll keep it in mind next time. |
|
no problem, thank you! |
gavv
added
duplicate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a version-check to CI checks. It takes the version fetched from git and from the file
version.hand compares it.#497