Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add report generation scripts #28

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Conversation

n8-dev
Copy link
Collaborator

@n8-dev n8-dev commented Nov 15, 2023

PHP version: 8.0+

Description

Adding two new scripts:

  1. bespoke-phpcs-report-source
  2. bespoke-phpcs-report-summary

Reason for the change

A new usecase from @poyjavier was the ability to see how other PHP code adhears to our coding standards as code review is sometimes a task done.

The ability to do the following

  1. Add this project as a dependancy
  2. Run a script to get the report
  3. Present said report

Will vastly streamline things and gives some reference-able data.

Note.

Ability to pass these commands dynamically can be looked at in the future.

Also, this is including the error of passed commands that is present in #27

Plan for Wiki update post merge:

Final checklist for reviewer

  • ReadMe updated with Sniff name and link to docs in Wiki
  • Unit test(s) added
  • Disallowed/Allowed code examples added

@n8-dev n8-dev requested a review from satrun77 November 16, 2023 01:42
Copy link
Contributor

@satrun77 satrun77 left a comment

Choose a reason for hiding this comment

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

@n8-dev
Copy link
Collaborator Author

n8-dev commented Nov 16, 2023

What do you mean @satrun77 ?
Its here?
https://github.com/silverstripeltd/bespoke-standards/pull/28/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34

FYIW: This is just a quick win for Innovation week.
Future goal will look at overhauling, param passing and setting.

@n8-dev n8-dev force-pushed the Add-report-generation-scripts branch from 534e68f to 92ae2fb Compare November 16, 2023 02:57
@satrun77
Copy link
Contributor

@n8-dev is there anything in here that we are waiting for? or is this ready for merge

@n8-dev
Copy link
Collaborator Author

n8-dev commented Mar 4, 2024

I want to look a rebuilding the CLI mechanics to properly support arguments at some point, but having these function as two additional commands for now is good enough, and brings the useful feature into the limelight.

I'd say good for an MVP.

So yes, ready for merge @satrun77

@n8-dev n8-dev force-pushed the Add-report-generation-scripts branch from 22f6d4c to 37563f3 Compare March 4, 2024 05:00
Copy link
Contributor

@satrun77 satrun77 left a comment

Choose a reason for hiding this comment

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

@satrun77 satrun77 merged commit 0d2d2bd into main Mar 5, 2024
3 checks passed
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