Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Add workflow to check config description #1894

Merged
merged 21 commits into from
Mar 4, 2025

Conversation

PeterChen13579
Copy link
Collaborator

fixes #1880

Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 73.18%. Comparing base (427ba47) to head (45cf6d2).
Report is 19 commits behind head on develop.

Files with missing lines Patch % Lines
src/util/newconfig/ConfigValue.hpp 80.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1894      +/-   ##
===========================================
+ Coverage    72.71%   73.18%   +0.46%     
===========================================
  Files          333      337       +4     
  Lines        13525    13822     +297     
  Branches      6881     7008     +127     
===========================================
+ Hits          9835    10115     +280     
+ Misses        1785     1780       -5     
- Partials      1905     1927      +22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@PeterChen13579 PeterChen13579 marked this pull request as draft February 14, 2025 03:40
@PeterChen13579 PeterChen13579 marked this pull request as ready for review February 17, 2025 05:02
Copy link
Collaborator

@cindyyan317 cindyyan317 left a comment

Choose a reason for hiding this comment

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

Logic looks good to me.

I am not against adding generated file to repo. I think it is better to give developer enough instruction how to update this generated file. Please ignore my comment if there is instruction somewhere 😄
For example: we can
1 Give instruction when check fails in CI job
2 Create a CMake target which depends on ConfigDescription.cpp file, it will update the markdown file during building when ConfigDescription.cpp is amended.
3 At the beginning of the config-description.md, we add This is generated file, don't amend it manually , please run balabala

PeterChen13579 and others added 15 commits February 18, 2025 12:06
Improving array parsing in config:
- Allow null values in arrays for optional fields
- Allow empty array even for required field
- Allow to not put an empty array in config even if array contains
required fields
Fixes XRPLF#1919. Please review and commit clang-tidy fixes.

Co-authored-by: godexsoft <385326+godexsoft@users.noreply.github.com>
Fixes XRPLF#1924. Please review and commit clang-tidy fixes.

Co-authored-by: godexsoft <385326+godexsoft@users.noreply.github.com>
There was a data race inside `CoroutineGroup` because internal timer was
used from multiple threads in the methods `asyncWait()` and
`onCoroutineComplete()`. Changing `registerForeign()` to spawn to the
same `yield_context` fixes the problem because now the timer is accessed
only from the same coroutine which has an internal strand.

During debugging I also added websocket support for `request_gun` tool.
Copy link
Collaborator

@kuznetsss kuznetsss left a comment

Choose a reason for hiding this comment

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

👍

@PeterChen13579 PeterChen13579 merged commit 86e2cd1 into XRPLF:develop Mar 4, 2025
22 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check Config Description in CI
5 participants