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

Psr4 compliant #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

unomateo
Copy link

Description of the Change

Renamed the RESO class to Reso for psr4 compliance. The class name needs to be the same as the file name, including capitalization. The examples worked fine when I was testing the code, but when I imported into a symfony PHP project, I received the error regarding psr4 autoloading similar to issue: #9 (comment)

Also added a new cli example for connecting to the bridge web api using only the server token provided by bridge.

Benefits

Ability to autoload in projects that require psr4 autoloading

Possible Drawbacks

None

Verification Process

After importing to symfony project and getting the error. I renamed the class file and dumped the composer autoload file. The result was no errors and I was able to make a connection to the web api and pull property data.

Applicable Issues

None

@programmer-man
Copy link

Can we get this merged please?

@ConnectGrid
Copy link

ConnectGrid commented Mar 5, 2024

PSR4 Specs state:

  • Alphabetic characters in the fully qualified class name MAY be any combination of lower case and upper case.
  • All class names MUST be referenced in a case-sensitive fashion.

So RESO\RESO was compliant. The only problem was that the class' FILE name was Reso.php when it should have been RESO.php to be compliant.

I'm personally not a fan of the Abc\Xyz case convention that has become so prevalent. If a name is correctly written as "RESO", then that's how it should be written everywhere, not "Reso". Same as "WebAPI", not "WebApi". Acronyms should be recognized as such. Just my take.

Would you reconsider this, reverse the pull request, and instead rename the file to RESO.php to satisfy the autoloader?

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