Closes #978

[NathanGibbs3]

Fixes Issue #978

[dregad]

Thanks for your contribution. Review comments:

Please follow our coding guidelines (PSR-2 / 12) in submitted code.

This is a bug, so should be targeted at hotfix/5.22 branch; if you could change target and rebase your branch accordingly, it would be great.

[NathanGibbs3]

I knew there would be some changes, working on those.

Please follow our coding guidelines (PSR-2 / 12) in submitted code.

Is there a PHPCodeSniffer rule set somewhere in the repo that I can run against my changes to make sure they are aligned with your coding standards?
I am not very familiar with the PSR standards, so having something to auto check things would be great.

This is a bug, so should be targeted at hotfix/5.22 branch; if you could change target and rebase your branch accordingly, it would be great.

Not a Git Ninja here, I may need a little help.
I need to figure out the following.

• How to pull the hotfix/5.22 branch into my github fork of ADOdb, so that I can pull it down locally
• I have no idea what you meant about rebasing my branch.
• Change the target branch of this PR.

I don't know what I don't know, so I'm letting you know what I don't know.
I'll try to figure it all out.

[NathanGibbs3]

I think I have all of the code specific changes, I just need to figure out how to get them to the right branch. and adjust this PR.

[dregad]

Is there a PHPCodeSniffer rule set somewhere

That's a work in progress. You'll find the current state of things at https://github.com/dregad/ADOdb/blob/codesniffer/phpcs.xml.dist if that helps.

At the moment the code base is not clean enough that I can easily run the tool against it, and it's a low-priority thing so...

How to pull the hotfix/5.22 branch into my github fork of ADOdb, so that I can pull it down locally

git remote add upstream git@github.com:ADOdb/ADOdb.git
git fetch upstream

I have no idea what you meant about rebasing my branch.

Your feature branch is based on master. The idea is to move the branch to be on top of hotfix/5.22 instead. There's several ways to do this, what I usually do is git rebase <current tip> <feature branch> --onto <target branch>, so for you probably something like this should do the trick

git rebase master NathanGibbs3:NathanGibbs3-Issue#978 --onto hotfix/5.22

Then (assuming you have the branch tracking your Github fork):

git push --force 

Change the target branch of this PR.

That's the easiest part, just click the Edit button next to the pull request's title and change the base

Let me know if you need more help

[dregad]

And if that sounds too difficult, forget about it I can do it myself too

[NathanGibbs3]

I was able to add upstream, but can't fetch it.

git fetch upstream

Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error: Could not fetch upstream

[NathanGibbs3]

And if that sounds too difficult, forget about it I can do it myself too

I value the learning experience. Not an issue.
It's part of git that I haven't used before.

[NathanGibbs3]

I was able to add upstream, but can't fetch it.

git fetch upstream

Permission denied (publickey). fatal: Could not read from remote repository.

Please make sure you have the correct access rights and the repository exists. error: Could not fetch upstream

Got it needed to git remote add upstream https://github.com/ADOdb/ADOdb.git

[NathanGibbs3]

I "think" we are good to go.
Give my work another once over before merging, if there are still issues, I'll fix them.
Do I need to remove the error suppressor from my code?
That's all I can think of at the moment.

[NathanGibbs3]

That's a work in progress. You'll find the current state of things at https://github.com/dregad/ADOdb/blob/codesniffer/phpcs.xml.dist if that helps.

At the moment the code base is not clean enough that I can easily run the tool against it, and it's a low-priority thing so..

Here is a piece of phpcs.xml that may help based on the conversations here & the .editorconfig file from the repo.

    <arg name="tab-width" value="4"/>
    <rule ref="Generic.WhiteSpace">
        <exclude name="Generic.WhiteSpace.DisallowTabIndent.TabsUsed" />
        <rule ref="Generic.WhiteSpace.ScopeIndent">
            <properties>
                <property name="tabIndent" value="true" />
            </properties>
        </rule>
    </rule>
    <rule ref="Generic.Files.LineLength">
        <properties>
            <property name="lineLimit" value="120" />
            <property name="absoluteLineLimit" value="120" />
        </properties>
    </rule>

[dregad]

Give my work another once over before merging, if there are still issues, I'll fix them.

Will do. Thanks for your efforts, I appreciate the collaboration on this

Do I need to remove the error suppressor from my code?

No, with the fix for #981 it's necessary to prevent an exception from being thrown when the SELECT lastval() fails.

[dregad]

• Missing declaration for $ADODB_GETONE_EOF global
• A few coding guidelines issues
• A comment explaining the reason for using @ would be useful

I'll fix those myself with a follow-up commit.

Thanks

[NathanGibbs3]

• Missing declaration for $ADODB_GETONE_EOF global

I can't believe I missed that. 🤦‍♂️

• A few coding guidelines issues

• A comment explaining the reason for using @ would be useful

I'll fix those myself with a follow-up commit.

OK, I see what you mean.
I checked out your follow up commit. I will keep that in mind if I submit any PR's in the future.
Anything to make merging easier.

Thanks

You are most welcome, thank you for maintaining ADOdb. 😄
Also, thank you for the pointers on git rebase, etc. I learned a few things today. 👍 😄