feat!: move skip-members to YML list

[huang-julien]

close #10

OMG @action/core returns string even for yml list.

Summary by CodeRabbit

• Documentation

  • Updated skip-members input to accept YAML list format (e.g., - username) instead of comma-separated values.

• Tests

  • Added test coverage for YAML list format parsing and validation of unsupported input formats.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0839d4f9-6b99-490b-b18d-7b21ffe69e30

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The pull request updates skip-members input parsing from comma-separated string format to YAML-style dash-list format. Changes include updated documentation, modified action definition, new parsing implementation, and corresponding test coverage validating the new format and ensuring incompatible formats are rejected.

Changes

Cohort / File(s)	Summary
Documentation README.md, action.yml	Updated skip-members input documentation and examples to reflect YAML list format instead of comma-separated strings.
Implementation src/index.ts	Added parseSkipMembers() helper function to parse newline-delimited YAML-style lists (lines starting with - ), replacing previous comma-splitting logic.
Test Suite src/index.test.ts	Updated test inputs with new agent-scan-comment flag; replaced comma-separated skip-members test with YAML dash-list format; added robustness tests verifying comma-separated and JSON array formats do not skip, and dash-list format correctly identifies members to skip.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A hop through the YAML list today,
No more commas in the fray!
With dashes bold, the members align,
Skip-members parsing is refined!
The tests confirm, it's crystal clear,
This breaking change we all should cheer! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat!: move skip-members to YML list' accurately describes the primary change - converting skip-members from comma-separated string to YAML list format.
Linked Issues check	✅ Passed	The PR successfully addresses issue #10 by implementing YAML list parsing for skip-members, fixing the string-inclusion bug and improving handling of multiple members.
Out of Scope Changes check	✅ Passed	All changes are directly related to the skip-members YAML list conversion objective; no unrelated modifications detected in documentation, action config, and code.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/index.ts (1)

58-69: ⚠️ Potential issue | 🟡 Minor

Normalize username casing before skip-list comparison.

Current matching is case-sensitive, so skip entries can fail when casing differs. Normalize both parsed entries and context.actor before includes().

💡 Proposed fix

-    const skipMembers = parseSkipMembers(skipMembersInput);
+    const skipMembers = parseSkipMembers(skipMembersInput).map((member) =>
+      member.toLowerCase(),
+    );
@@
-    if (skipMembers.includes(username)) {
+    if (skipMembers.includes(username.toLowerCase())) {
       core.info(`Skipping analysis for ${username}`);
       return;
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/index.ts` around lines 58 - 69, The skip-list comparison is
case-sensitive: normalize casing for both the parsed skip list and the current
actor before calling includes(). Update parseSkipMembers or its usage so
skipMembers contains lowercased entries (e.g., mapping entries to
.toLowerCase()), and normalize context.actor (username) to the same case before
checking skipMembers.includes(username); refer to parseSkipMembers,
skipMembersInput, skipMembers, and github.context (context.actor) when making
the change.

🧹 Nitpick comments (2)

README.md (1)

38-55: Clarify migration and accepted skip-members syntax explicitly.

Please add a short note here that comma-separated and JSON-array formats are no longer honored, and only dash-prefixed multiline entries are supported. This will reduce upgrade confusion for existing users.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` around lines 38 - 55, Update the README's "skip-members" section
to explicitly state that the accepted syntax has changed: only YAML
dash-prefixed multiline lists (as shown under the `skip-members` example) are
supported now, and previous comma-separated strings or JSON-array formats are no
longer honored; mention `skip-members` by name and clarify migration steps for
users to convert old formats into the dash-prefixed multiline form to avoid
confusion during upgrades.

src/index.test.ts (1)

292-302: This skip test overlaps heavily with the earlier YAML skip case.

Consider merging this with the test at Line 246 to reduce duplication and keep intent focused.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/index.test.ts` around lines 292 - 302, The new test "should skip analysis
for member in dash-prefixed YAML list" duplicates behavior already covered by
the earlier YAML skip test; merge them by expanding the existing test (the one
that calls setupInputs and run around Line 246) to also assert against the
dash-prefixed YAML variant rather than keeping a separate spec. Concretely,
update the earlier test that uses setupInputs({ "skip-members": ... }) and run()
to include an additional sub-case or param (e.g., add a second setupInputs
invocation or loop inputs) that supplies "- other-user\n- test-user" and keep
the same expectations against core.info, github.getOctokit, and
identifyReplicant so you remove the redundant it(...) block and retain a single
test covering both YAML formats.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/index.ts`:
- Around line 58-69: The skip-list comparison is case-sensitive: normalize
casing for both the parsed skip list and the current actor before calling
includes(). Update parseSkipMembers or its usage so skipMembers contains
lowercased entries (e.g., mapping entries to .toLowerCase()), and normalize
context.actor (username) to the same case before checking
skipMembers.includes(username); refer to parseSkipMembers, skipMembersInput,
skipMembers, and github.context (context.actor) when making the change.

---

Nitpick comments:
In `@README.md`:
- Around line 38-55: Update the README's "skip-members" section to explicitly
state that the accepted syntax has changed: only YAML dash-prefixed multiline
lists (as shown under the `skip-members` example) are supported now, and
previous comma-separated strings or JSON-array formats are no longer honored;
mention `skip-members` by name and clarify migration steps for users to convert
old formats into the dash-prefixed multiline form to avoid confusion during
upgrades.

In `@src/index.test.ts`:
- Around line 292-302: The new test "should skip analysis for member in
dash-prefixed YAML list" duplicates behavior already covered by the earlier YAML
skip test; merge them by expanding the existing test (the one that calls
setupInputs and run around Line 246) to also assert against the dash-prefixed
YAML variant rather than keeping a separate spec. Concretely, update the earlier
test that uses setupInputs({ "skip-members": ... }) and run() to include an
additional sub-case or param (e.g., add a second setupInputs invocation or loop
inputs) that supplies "- other-user\n- test-user" and keep the same expectations
against core.info, github.getOctokit, and identifyReplicant so you remove the
redundant it(...) block and retain a single test covering both YAML formats.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9f30789b-7c21-4817-8d65-3883a5c95e12

📥 Commits

Reviewing files that changed from the base of the PR and between 6b02b79 and dfac5ff.

📒 Files selected for processing (4)

• README.md
• action.yml
• src/index.test.ts
• src/index.ts

[MatteoGabriele]

We should probably move this to v2 as well. I created a v2 branch where we can include all breaking change items and release it later.

[MatteoGabriele]

@huang-julien, after you push this one to the v2 branch, I will update the report feature as well, so we also use YAML-style lists. Right now, it's just using a string-array since it only accepts strings.