Hi :wave: How can I get grant to push my feature branch to Signoz/signoz? I created <this PR> but Je...
a
Hi šŸ‘‹ How can I get grant to push my feature branch to Signoz/signoz? I created this PR but Jest Coverage CI fails because the PR is created from my forked repo.
n
@Prashant Shahi ^
a
would you let me know when you've gave me permission? @Prashant Shahi
would you help me about this because he doesn't respond? @nitya-signoz
n
let me check
p
Hi @Akira Hayashi, you can ignore the jest coverage test - it is not mandatory for PR merges. Do you need to expose the query-service port for the cold storage test? I see the same network is used
signoz_test
. Also, why a different workflow for the cold storage test? Why not include this as part of build workflow under query-service and query-service-ee sections? @nitya-signoz can you please help review the go tests and related files?
a
Thank you!! I'll review my changes with your advice šŸ‘
Thank you, I checked my codes:
Do you need to expose the query-service port for the cold storage test? I see the same network is used
signoz_test
.
yes, the exposed ports are needed to keep auth endpoints work, not for cold storage test. I got
Post "<http://localhost:8180/api/v1/login>": dial tcp [::1]:8180: connect: connection refused
if I quit exposing the ports @Prashant Shahi
Also, why a different workflow for the cold storage test? Why not include this as part of build workflow under query-service and query-service-ee sections?
yeah, you're right!! I forgot to consider the way you suggest. I'll delete the new workflow I added, and run tests before build processes. Thank you @Prashant Shahi
> why a different workflow for the cold storage test? Why not include this as part of build workflow under query-service and query-service-ee sections? • I removed the unnecessary workflow I added • instead, I executed ALL the query-service tests in the build pipeline Thank you for your advice! @Prashant Shahi https://github.com/SigNoz/signoz/pull/6098/commits/a5e84a5729a243caf3e6c0e2a1750b9724a0c50a
about: https://github.com/SigNoz/signoz/pull/6098#issuecomment-2408808309 would you tell me the blocking problem of this PR? (if it could, I know it is difficult if the problem is an internal one) @Srikanth Chekuri
s
Hi @Akira Hayashi, Thanks for the PR. I wanted to hold on to the PR to check if you should be sorting in the tests or if the sorting should happen automatically.
a
ah, simply my code is still needed to be reviewed! I understand, thank you! @Srikanth Chekuri
a
It merged! Thank you all for your supports!