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

Upstream codereview changes #15

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Upstream codereview changes #15

wants to merge 10 commits into from

Conversation

svandragt
Copy link
Owner

@svandragt svandragt commented Jan 9, 2024

We've used the plugin on a project and I've upstreamed code review changes.

  1. Updated HTMX and deferred loading it.
  2. Function documentation improvements
  3. return types were added.
  4. Sanitization of nonce checking
  5. Separation of concerns in the template loading logic, including removal of the "partial" loading of templates, which did not currently bypass the WordPress logic anyway.
  • test changes
  • test 404
  • restore demo template registration regression
  • documentation is up to date

The PHP code has been refactored to adhere to PSR-1 Coding Standards. This includes adding comprehensive function docblocks for 'activate', 'deactivate', and 'bootstrap' functions, as well as correcting the structure to allow for PHP CodeSniffer's PSR1 rules. Furthermore, PHP CodeSniffer's warning for side effects has been ignored on the first line of script.
# Conflicts:
#	htmxpress.php
@svandragt svandragt added the help wanted Extra attention is needed label Jan 9, 2024
@svandragt svandragt self-assigned this Jan 9, 2024
@svandragt svandragt removed the help wanted Extra attention is needed label Jan 10, 2024
@svandragt svandragt marked this pull request as draft January 10, 2024 08:53
@svandragt
Copy link
Owner Author

Needs tested still.

Comment on lines -59 to -61
if (empty($paths)) {
$paths[] = dirname( __FILE__, 2 ) . "/templates/";
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

Oops I removed the demo templates registration here.

$wp_query->set_404();
status_header( 404 );
}
return $template_loaded;
Copy link
Owner Author

@svandragt svandragt Jan 10, 2024

Choose a reason for hiding this comment

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

The code is not testing for 404 here, verify this is working correctly.

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.

1 participant