Skip to content

fix(api): drop stale publish-no-media rule and pinning test#515

Open
staging-devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1779719243-fix-publish-no-media-doc
Open

fix(api): drop stale publish-no-media rule and pinning test#515
staging-devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1779719243-fix-publish-no-media-doc

Conversation

@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor

@staging-devin-ai-integration staging-devin-ai-integration Bot commented May 25, 2026

Summary

  • The publish-no-media rule was documented in the lint_client_section rustdoc but never implemented — its surface is covered by the empty-tracks rule. The rustdoc was cleaned up in b5c2eae, but the pinning test publish_no_media_rule_is_not_emitted remained.
  • Remove the now-unnecessary pinning test from client_lint.rs.
  • Rename test_lint_publish_no_mediatest_lint_empty_tracks in tests.rs to match the actual rule it validates.

Closes #473

Review & Validation

  • just lint and cargo test -p streamkit-api pass with no regressions.

Link to Devin session: https://staging.itsdev.in/sessions/65938fd28fa94a968db2a17d67954a4e
Requested by: @streamer45


Devin Review

Status Commit
🟢 Reviewed 7a66ff7
Open in Devin Review (Staging)

The publish-no-media rule was documented in the client_lint rustdoc but
never implemented — its surface is already covered by the empty-tracks
rule. The rustdoc was cleaned up in b5c2eae, but the pinning test
remained.

- Remove the publish_no_media_rule_is_not_emitted pinning test from
  client_lint.rs (no longer needed since the doc no longer claims the
  rule exists).
- Rename test_lint_publish_no_media to test_lint_empty_tracks in
  tests.rs to match the actual rule it validates.

Closes #473

Signed-off-by: streamkit-devin <devin@streamkit.dev>
@staging-devin-ai-integration
Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

Copy link
Copy Markdown
Contributor Author

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review (Staging)
Debug

Playground

Comment on lines +847 to 850
fn test_lint_empty_tracks() {
let mut c = dynamic_client();
c.publish = Some(PublishConfig { broadcast: "x".into(), tracks: vec![] });
let warnings = lint_client_section(&c, EngineMode::Dynamic);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

📝 Info: Deleted regression test overlapped with the renamed empty-tracks coverage

The removed client_lint.rs unit test only asserted that a normal publish config with one track does not emit the obsolete publish-no-media rule. Since the lint implementation emits empty-tracks only when publish.tracks.is_empty() (crates/api/src/yaml/client_lint.rs:169-175), the renamed test here continues to cover the active behavior for empty publish configurations. A repository-wide search found no remaining publish-no-media references, so this looks like test cleanup rather than a behavioral change that needs a bug finding.

(Refers to lines 847-851)

Open in Devin Review (Staging)

Was this helpful? React with 👍 or 👎 to provide feedback.

Debug

Playground

@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.40%. Comparing base (b2d01c2) to head (7a66ff7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #515      +/-   ##
==========================================
+ Coverage   79.39%   79.40%   +0.01%     
==========================================
  Files         232      232              
  Lines       66904    66883      -21     
  Branches     1909     1909              
==========================================
- Hits        53117    53109       -8     
+ Misses      13781    13768      -13     
  Partials        6        6              
Flag Coverage Δ
backend 79.13% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 85.27% <ø> (ø)
engine 83.59% <ø> (-0.10%) ⬇️
api 89.79% <ø> (-0.20%) ⬇️
nodes 75.38% <ø> (+0.04%) ⬆️
server 80.27% <ø> (+0.01%) ⬆️
plugin-native 83.47% <ø> (ø)
plugin-wasm 91.90% <ø> (ø)
ui-services 84.67% <ø> (ø)
ui-components 60.49% <ø> (ø)
Files with missing lines Coverage Δ
crates/api/src/yaml/client_lint.rs 94.30% <ø> (-0.25%) ⬇️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

client_lint: 'publish-no-media' rule is documented but not implemented

2 participants