Skip to content

Add StructArmed to QA#123

Closed
samsonasik wants to merge 5 commits into
typesense:masterfrom
samsonasik:add-structarmed-to-qa
Closed

Add StructArmed to QA#123
samsonasik wants to merge 5 commits into
typesense:masterfrom
samsonasik:add-structarmed-to-qa

Conversation

@samsonasik
Copy link
Copy Markdown
Contributor

Change Summary

Hi, this PR adds StructArmed to github workflow static analysis for QA.

https://github.com/boundwize/structarmed

StructArmed is a configurable PHP architecture guard, that we can define layers and rules, then keep them enforced.

For starter, this PR uses the PSR4 preset, and already found violations that fixed and included in this PR.

Since it support php 7.x as well, this only add on github workflow on php 8.4 👍

PR Checklist

@samsonasik
Copy link
Copy Markdown
Contributor Author

Ready to review/merge 👍

@tharropoulos
Copy link
Copy Markdown
Collaborator

Looks like there's a step that's failing:

https://github.com/typesense/typesense-php/actions/runs/26855395671/job/79324002208?pr=123#step:5:1

Any specific reason why you're omitting it from the dependencies of the project and only adding it as a dependency under the pipeline?

@samsonasik
Copy link
Copy Markdown
Contributor Author

@tharropoulos StructArmed require php 8.2+, this project still allow php 7.× so it can't directly uses in composer.json.

Probably some permission needed to write composer.lock on github workflow or need docker setup like the main tests workflow.

@tharropoulos
Copy link
Copy Markdown
Collaborator

What's the difference between using this and just using

composer dump-autoload --optimize --strict-psr --strict-ambiguous

@samsonasik
Copy link
Copy Markdown
Contributor Author

Composer may warn about PSR-4 autoloading issues, but it does not really enforce them as architecture rules for the project.

StructArmed is not only about PSR autoloading/naming conventions, it just one of the presets, it can detect when some class or layer should not use another class from another namespace/layer, you can check how practical it is:

https://github.com/codeigniter4/CodeIgniter4/blob/develop/structarmed.php

This PR is for the starter only to give quick entrance.

@samsonasik
Copy link
Copy Markdown
Contributor Author

samsonasik commented Jun 3, 2026

@tharropoulos I've updated github workflow which hopefully fix the permission issue 👍

Could you try run github workflow? Thank you.

Btw, if you need another example of how set the config layer -> ruleset for more complex use, you can take a look on how cakephp and codeigniter4 uses it :)

@tharropoulos
Copy link
Copy Markdown
Collaborator

Thanks for the detailed write-up and the CodeIgniter4 link, really appreciate it. The test case changes themselves look solid, and if you want to split them off into their own PR I'm happy to merge those right away.

Since you appear to be the primary/sole contributor to StructArmed, please disclose that relationship when proposing it as a new CI dependency. We evaluate third-party tooling differently when the PR author also maintains the tool, especially for a new pre-1.0 package.

On StructArmed itself I'm hesitant though. Most of the value in that CodeIgniter4 config comes from enforcing rules across thirty-plus layers, which is a much bigger surface than what is here. typesense-php is a thin REST wrapper sitting in a single namespace, so I'm not sure what architecture rules we'd actually be enforcing day to day, and I'd rather not take on another tool to maintain unless there's a concrete problem it solves for us.

@samsonasik samsonasik mentioned this pull request Jun 3, 2026
1 task
@samsonasik
Copy link
Copy Markdown
Contributor Author

@tharropoulos sure, I created separate PR to only apply the psr-4 fixes:

@samsonasik
Copy link
Copy Markdown
Contributor Author

Let's close this for now :)

@samsonasik samsonasik closed this Jun 3, 2026
@samsonasik samsonasik deleted the add-structarmed-to-qa branch June 3, 2026 21:10
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.

2 participants