Skip to content

feat(allowlist): add matlab support - #574

Open
GenosseFlosse wants to merge 3 commits into
alibaba:mainfrom
GenosseFlosse:main
Open

feat(allowlist): add matlab support#574
GenosseFlosse wants to merge 3 commits into
alibaba:mainfrom
GenosseFlosse:main

Conversation

@GenosseFlosse

@GenosseFlosse GenosseFlosse commented Jul 29, 2026

Copy link
Copy Markdown

Description

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (no functional changes)
  • Documentation update
  • CI / Build / Tooling

How Has This Been Tested?

  • make test passes locally
  • Manual testing (describe below)

Checklist

  • My code follows the project's coding style (go fmt, go vet)
  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated the documentation accordingly (if applicable)
  • I have signed the CLA

Related Issues

@CLAassistant

CLAassistant commented Jul 29, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: No comments generated. Looks good to me.

@GenosseFlosse GenosseFlosse changed the title adding matlab rules feat(allowlist): add matlab support Jul 29, 2026
@GenosseFlosse
GenosseFlosse marked this pull request as ready for review July 29, 2026 08:04

@lizhengfeng101 lizhengfeng101 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the contribution — the MATLAB rule doc itself is excellent and very thorough!

One concern before merging: the .m extension is shared between MATLAB and Objective-C. This PR maps all **/*.m files to matlab.md, which means Objective-C files (e.g. ios/ViewController.m, AppDelegate.m) will receive MATLAB-specific review feedback — clearly incorrect for those files.

The test change confirms this: ios/ViewController.m was removed from the "fallback to default" test because it now matches the MATLAB rule.

Possible approaches

  1. Path-based heuristics — put ObjC-specific glob patterns (e.g. **/{ios,macos,tvos,watchos,Pods,Classes}/**/*.m) before **/*.m in system_rules.json, leveraging "first match wins" order. Downside: incomplete coverage, maintenance burden.

  2. Content-based detection — sniff the first line of the file (function → MATLAB, #import/#include → ObjC). Downside: requires changes to the Resolve interface which currently only takes a path.

  3. Accept the limitation — document that .m defaults to MATLAB and ObjC projects should use a project-level .opencodereview/rule.json to override. Simplest, but shifts burden to ObjC users.

Minor formatting issues in system_rules.json

  • Trailing space on the "**/*.bicep": "bicep.md", line
  • The new "**/*.m" entry uses a tab for indentation, while the rest of the file uses 4 spaces

Would love to hear your thoughts on how you'd like to handle the extension ambiguity. Happy to discuss further!

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.

3 participants