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

Improve Scrutinizer Score #675

Closed
DevinWalker opened this issue Jun 30, 2016 · 3 comments
Closed

Improve Scrutinizer Score #675

DevinWalker opened this issue Jun 30, 2016 · 3 comments
Assignees

Comments

@DevinWalker
Copy link
Member

DevinWalker commented Jun 30, 2016

They provide a lot of great code improvement fixes that we should be completing ongoing as we can on Scrutinizer: https://scrutinizer-ci.com/g/WordImpress/Give/code-structure/master/hot-spots

@DevinWalker DevinWalker self-assigned this Jun 30, 2016
DevinWalker pushed a commit that referenced this issue Jun 30, 2016
@DevinWalker DevinWalker removed the Epic label Jul 29, 2016
@ravinderk ravinderk mentioned this issue Jul 29, 2016
DevinWalker pushed a commit that referenced this issue Jul 29, 2016
DevinWalker pushed a commit that referenced this issue Aug 15, 2016
…h default fallback to improve scrutinizer score #675
DevinWalker pushed a commit that referenced this issue Aug 15, 2016
* master:
  Doc added for $default param
  Remove usage of global $give_options in place for give_get_option with default fallback to improve scrutinizer score #675

# Conflicts:
#	includes/misc-functions.php
@ramiy
Copy link
Contributor

ramiy commented Aug 20, 2016

Interesting read about globals and how to remove them from your code: https://core.trac.wordpress.org/ticket/37699

DevinWalker pushed a commit that referenced this issue Aug 31, 2016
DevinWalker pushed a commit that referenced this issue Aug 31, 2016
DevinWalker pushed a commit that referenced this issue Aug 31, 2016
DevinWalker pushed a commit that referenced this issue Sep 1, 2016
DevinWalker pushed a commit that referenced this issue Sep 8, 2016
* master: (24 commits)
  New POT file generated for 1.6.1
  Added change log for #863
  Added code to set default email receipt content upon install #863
  version bumped to 1.6.1
  Change log added for #945
  Add `first_name` and `last_name` to PP standard #945
  Fixed screenshot description issue #990
  Change log item added for #997
  php docs
  Fix issue #997
  Add extra class attribute
  Fixed @property-read conflicting with calls to read-only props and added missing props #675
  Addiction @property-read doc block #675
  Removed give_undo_donation_on_refund which is never used - left over from fork - code improved #675
  Add @property-read to class declaration to resolve https://scrutinizer-ci.com/g/WordImpress/Give/inspections/f8e0f43d-ff3d-4e85-9fbd-ac6be87fac72/issues/files/includes/emails/class-give-email-tags.php?status=new&orderField=path&order=asc&honorSelectedPaths=0 #675
  Simplify logic & Scrutinizer score #675
  meta doesn't exist in Give_Payment - it's payment_meta
  Use missing  parameter
  Fix the title level text in {donation} email template tag
  Fix issue #981
  ...

Fixed Conflicts:
	give.php
	includes/admin/reporting/export/class-batch-export-customers.php
	includes/gateways/paypal-standard.php
	includes/install.php
	includes/payments/class-give-payment.php
	languages/give.pot
	readme.txt
	templates/shortcode-profile-editor.php
DevinWalker pushed a commit that referenced this issue Nov 18, 2016
DevinWalker pushed a commit that referenced this issue Nov 21, 2016
… issue/1247

* 'issue/1247' of https://github.com/WordImpress/Give:
  Provide properties for PHPdocs related to #675
  Refactored give_email_tag_donation  #1247

# Conflicts:
#	includes/emails/class-give-email-tags.php
DevinWalker pushed a commit that referenced this issue Nov 21, 2016
* release/1.7: (22 commits)
  Removed the default output text “There was an error retrieving”… #1247
  Changed var naming from $include_level_name to $only_level for better clarity #1247
  Updated Settings > licencing CSS layout #357
  Style update for licenses
  Consolidate offline donation gateway functions into the offline gateway file #1261 #1262
  Sentence closure
  Revert "Add line and file name to error notice"
  update notice message
  Add line and file name to error notice
  add filter hook show notice for offline donation
  update pending donation notice
  define missing
  remove unnessary class from offline instruction row from receipt
  update reciept css
  update offline payment message on receipt page
  add action hook to add offline instruction to receipt
  refactor give_offline_payment_cc_form function
  Add helper function give_get_offline_payment_instruction
  Improve PHPdocs #675
  Provide properties for PHPdocs related to #675
  ...

# Conflicts:
#	assets/css/give-admin-rtl.css
#	assets/css/give-admin-rtl.min.css
#	assets/css/give-admin.css
#	assets/css/give-admin.min.css
#	assets/scss/admin/settings.scss
#	assets/sourcemaps/give-admin.css.map
#	assets/sourcemaps/give.css.map
#	includes/admin/class-give-settings.php
#	includes/class-give-donate-form.php
#	includes/gateways/offline-donations.php
#	templates/give-rtl.min.css
#	templates/give.min.css
a
DevinWalker pushed a commit that referenced this issue Dec 7, 2016
DevinWalker pushed a commit that referenced this issue Dec 7, 2016
Improving doc blocks / Scrutinizer #675
DevinWalker pushed a commit that referenced this issue Dec 8, 2016
* master:
  New POT file for 1.7
  Added additional readme items for 1.7
  Fixed typos and doc bloc inaccuracies #675
  Additional property declarations #675
  Add form_id and form object to give_form_title filter
  Do not render unpublish or transhed form
  Add form id and form object to give_form_title

# Conflicts:
#	includes/forms/template.php
@DevinWalker
Copy link
Member Author

DevinWalker commented Dec 8, 2016

@ravinderk let me run this idea by you. We're getting hit pretty hard on Scrutinizer due to the code complexity "Conditions" and "Paths":

2016-12-07_21-10-23
2016-12-07_21-10-23

Link: https://scrutinizer-ci.com/g/WordImpress/Give/code-structure/master/operation/includes%2Fgateways%2Fpaypal-standard.php%3A%3Agive_process_paypal_payment

This example has >20000: https://scrutinizer-ci.com/g/WordImpress/Give/code-structure/master/operation/includes%2Fprocess-donation.php%3A%3Agive_purchase_form_validate_new_user

Proposed Solution

For the "Paths" it's likely due to checking if isset() in a ternary conditional. Why don't we create a function that we could pass all these conditonals through, or does one already exist in Core that I'm unaware of? We are mainly doing all these isset() and empty() conditionals to prevent PHP notices.

/**
 * Check variable.
 *
 * Helper function to check if a variable is set, empty, etc.
 *
 * @param        $variable
 * @param string $conditional
 * @param        $default
 *
 * @return mixed
 */
function give_check_variable( $variable, $conditional = 'isset', $default ) {

	if ( $conditional == 'isset' ) {
		$variable = isset( $variable ) ? $variable : $default;
	} elseif ( $conditional = 'empty' ) {
		$variable = empty( $variable ) ? $variable : $default;
	} elseif ( $conditional = 'null' ) {
		$variable = is_null( $variable ) ? $variable : $default;
	}

	return $variable;

}

This was referenced Dec 8, 2016
DevinWalker pushed a commit that referenced this issue Jan 17, 2017
* master:
  Fixed indentation issue
  Trying again…
  Fixing scrutinizer upset with brackets {} :(
  Ensure scrutinizer uses WP coding standards when sniffing #675
  Version bumped to 1.7.2 - readme updated for release - POT file generated

Fixed Conflicts:
	.scrutinizer.yml
	give.php
	readme.txt
DevinWalker pushed a commit that referenced this issue Jan 23, 2018
Phpdoc cleanup to help with Scrutinizer in #675
@DevinWalker
Copy link
Member Author

Closing this old issue out as it's an ongoing effort.

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

No branches or pull requests

5 participants