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

Adds namespacing and makes function naming more consistent. #773

Merged
merged 69 commits into from
Jul 29, 2022

Conversation

batesweb
Copy link
Contributor

@batesweb batesweb commented Oct 18, 2021

Closes #769

DESCRIPTION

Removes the function prefixes (_s) and adds the WD_S namespace throughout the theme.
Updates function names to be more consistent. Functions that render markup start with display_ and functions that return data start with return_.

SCREENSHOTS

Doesn't change anything visual in the theme. Just refactors the existing code.

OTHER

  • Is this issue accessible? (Section 508/WCAG 2.0AA)
  • Does this issue pass all the linting? (PHPCS, ESLint, SassLint)
  • Does this pass CBT?

STEPS TO VERIFY

Test all theme functionality to make sure there are no namespacing issues.

DOCUMENTATION

Will this pull request require updating the wd_s wiki?

@batesweb batesweb changed the title Feature/#769 function naming Adds namespacing and makes function naming more consistent. Oct 18, 2021
@batesweb batesweb marked this pull request as ready for review October 18, 2021 18:37
Copy link
Contributor

@jrfoell jrfoell left a comment

Choose a reason for hiding this comment

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

I like where this is going! I think we need to discuss the actual namespace prefix(es) but that can be a group discussion 😎

inc/setup/preload-assets.php Outdated Show resolved Hide resolved
@coreymcollins coreymcollins added the documentation An update to documentation is required after a PR has been approved and merged in. label Nov 24, 2021
@batesweb batesweb mentioned this pull request Dec 30, 2021
3 tasks
Copy link
Contributor

@aubreypwd aubreypwd left a comment

Choose a reason for hiding this comment

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

I can neither approve or reject this, I think either way we go Namespacing is super helpful here. I also like the nested namespacing e.g. for wd_s\functions and wd_s\template_tags vs. just piling them all in a single wd_s namespace.

Please see my advise though, as I think we can format this a little better...

inc/template-tags/display-svg.php Outdated Show resolved Hide resolved
@coreymcollins
Copy link
Contributor

@batesweb – it looks like this branch is a bit out of date now, and I'm not sure the full status of these updates. When you get a chance, can you review this branch, bring it up to date, and set it R4R if it's ready for final testing & review? Thanks!

Copy link
Contributor

@jrfoell jrfoell left a comment

Choose a reason for hiding this comment

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

Liking these latest changes using print_ and get_ from the BEE feedback.

header.php Outdated Show resolved Hide resolved
header.php Show resolved Hide resolved
inc/customizer/customizer.php Show resolved Hide resolved
inc/functions/compat.php Outdated Show resolved Hide resolved
Copy link
Contributor

@oliverharrison oliverharrison left a comment

Choose a reason for hiding this comment

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

Looks good!

@oliverharrison oliverharrison merged commit fea624c into main Jul 29, 2022
@oliverharrison oliverharrison deleted the feature/#769-function-naming branch July 29, 2022 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation An update to documentation is required after a PR has been approved and merged in. In Progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use namespacing in place of function prefixes and make function names more consistent
8 participants