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

Made very small change for this to work in PHP 8.2 #42

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

Conversation

schucan
Copy link

@schucan schucan commented May 19, 2024

No description provided.

//- Check user authentication, login and logout
$auth = new Authorization(); //create authorization object

// check if user has attempted to log out
if (isset($_GET['logout']))
if (isset($_POST['logout']))
Copy link

Choose a reason for hiding this comment

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

But ... why? It really doesn't matter if this is a GET or a POST request. You could make the argument that it changes state on the server so it should be a POST request, but that's hardly ever enforced for session state type changes that only affect to a logout state. For a login state, sure, but logout?

@Dygear
Copy link

Dygear commented Jun 4, 2024

Your "Very Small Change" edits 4k lines. That's not at all a small change. Code formatting was also moved to a different style makes up a huge number of these changes. There's also a lot of strange white space added to the end of lines. What editor are you using? It's strange to change the style of functions (but could be because of auto formatting tools, but to then not trim trailing spaces from lines makes no sense to me.)

@schucan
Copy link
Author

schucan commented Jun 4, 2024

I am sorry, Mark @Dygear. I only saw after the pull request that someone else had posted a much better one and I couldn't figure out how I could retract mine.

Plus I didn't know whether anyone was ever looking at those pull requests.

I guess if you merge the other user's pull request, we're perfectly fine.

I am very happy with this tool as it is!

Thanks for sharing it!

@Dygear
Copy link

Dygear commented Jun 4, 2024

While I'm listed as a member, that's simply because I moved over the domains to them and also made this Github org for them for when I had my own version of PHPLiteAdmin before I knew this existed. Most of the work for phpLiteAdmin is actually done on bitbucket and this is just a mirror of that repo basically. I do not have permission to make merges. You should checkout bitbucket. I'm sure @crazy4chrissi should be around at some point to look these over.

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