Remove old config code, remove name space from variable name [#2737157]

Currently variable names are like social_stats.cron_settings.social_stats_url_root, since in Drupal 8 config variable names starts with module name, so we can safely name variable social_stats.cron_settings.url_root.
Added default values to some config variables in config schema file, $config->get() takes only one argument, removed second arg from get calls, default values for those variables is stored in config schema file.
Removed old settings form code.

Comment	File	Size	Author
#8	2737157-interdiff-1-5.txt	2.6 KB	Sharique
#6	remove_old_config_code-2737157-5.patch	13.97 KB	Sharique
#3	remove_old_config_code-2737157-3.patch	13.95 KB	Sharique
	config-cleanup.patch	13.92 KB	Sharique

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

31 May 2016 at 08:24

Sharique created an issue. See original summary.

Comment #2

vijaycs85

London, UK

CreditAttribution: vijaycs85 as a volunteer and at Capgemini commented 1 June 2016 at 11:50

+++ b/config/install/social_stats.settings.yml
@@ -0,0 +1,10 @@
+content_types_article:

I would go with

content_types:
  article:
    fb: true
    twitter: true
  page:
    fb: true
    twitter: true

Comment #3

Sharique CreditAttribution: Sharique as a volunteer commented 3 June 2016 at 12:30

File	Size
remove_old_config_code-2737157-3.patch	13.95 KB

Comment #4

vijaycs85

London, UK

CreditAttribution: vijaycs85 as a volunteer and at Capgemini commented 3 June 2016 at 12:34

+++ b/social_stats.module
@@ -25,20 +25,20 @@ function social_stats_help($route_name, RouteMatchInterface $route_match) {
+      $content_type_settings = $settings->get('content_types.' . $types->id());

nitpick: $content_type_settings = \Drupal::config('social_stats.settings).get('content_types);

then
we can check if (isset($content_type_settings[$type->id()]))

This will make the if condition lighter.

Comment #5

vijaycs85

London, UK

CreditAttribution: vijaycs85 as a volunteer and at Capgemini commented 3 June 2016 at 12:36

Status:

Needs review

» Reviewed & tested by the community

Anyway, it looks much better.

Comment #6

Sharique CreditAttribution: Sharique as a volunteer commented 3 June 2016 at 12:56

File	Size
remove_old_config_code-2737157-5.patch	13.97 KB

Did that nitpick and bit more.

Comment #7

AjitS

he/him

Marathi

Pune

CreditAttribution: AjitS as a volunteer and at QED42 commented 3 June 2016 at 13:24

Thank you @vijaycs85 for the reviews!

@Sharique : Could you please attach an interdiff so that the review is easy?

Comment #8

Sharique CreditAttribution: Sharique as a volunteer commented 3 June 2016 at 17:43

File	Size
2737157-interdiff-1-5.txt	2.6 KB

2 files were hidden/shown/deleted

File	Size
config-cleanup.patch	13.92 KB
remove_old_config_code-2737157-3.patch	13.95 KB

@Ajit here is interdiff, I already committed this code, don't know why it is not showing here, also not showing in commit list page https://www.drupal.org/node/2080003/commits.

Comment #9

AjitS

he/him

Marathi

Pune

CreditAttribution: AjitS as a volunteer and at QED42 commented 4 June 2016 at 06:12

Status:

Reviewed & tested by the community

» Fixed

Thank you! I'd assume that the commit not showing up here should be a temporary issue. However, if this continues we should create an issue in the webmasters issue queue.

Marking this issue as fixed, with appropriate credits. Thank you again!