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

Make logging optional, closes #47 #48

Merged
merged 3 commits into from
Apr 3, 2019

Conversation

sreichel
Copy link
Collaborator

@sreichel sreichel commented Apr 3, 2019

No description provided.

@Schrank
Copy link
Member

Schrank commented Apr 3, 2019

Das sieht klasse aus - ich fände es total gut, wenn man statt isLoggingEnabled überall prüft wir eine Helper Method einführen würden, die das logging übernimmt, also dann Mage::helper('honeyspam')->log()

Wenn du da Lust drauf hast das zu ändern, freue ich mich, wenn nicht, merge ich das auch so.

@sreichel
Copy link
Collaborator Author

sreichel commented Apr 3, 2019

NAAIN. Hatte ich vorher und habs zurückgedreht ... dachte "lass es lieber einfach" :)

sreichel@ac597d2

... ich passe es morgen nochmal an ...

@Schrank
Copy link
Member

Schrank commented Apr 3, 2019

Ne, ich meine das alles in den helper auslagern:

public function log()
	if ($this->isLoggingEnabled()) {
		Mage::log("SPAM: " . $param . " has at least 2 CAPITAL letters at the end", Zend_Log::INFO, 'honeyspam.log');
	}
}

Und dann in den verschiedenen Klassen nur noch einen Einzeiler:
$helper->log();

@sreichel
Copy link
Collaborator Author

sreichel commented Apr 3, 2019

Versteh schon, :) In der Checker.php sind die Log-Aufrufe alle in einer Methode, reicht es da nicht vorab zu prüfen, ob das Logging aktiviert ist?

@riconeitzel
Copy link
Contributor

riconeitzel commented Apr 3, 2019 via email

@sprankhub
Copy link
Member

Thanks @sreichel! Looks good! However, as @Schrank suggested, I would appreciate as well if you could add a helper log method, which checks if logging is enabled and only logs if it is enabled.

@sprankhub sprankhub self-requested a review April 3, 2019 14:43
@sprankhub sprankhub requested a review from Schrank April 3, 2019 14:44
@Schrank Schrank merged commit 8e3d35a into magento-hackathon:master Apr 3, 2019
@Schrank
Copy link
Member

Schrank commented Apr 3, 2019

<3 Danke!

@sreichel sreichel deleted the optional-logging branch April 3, 2019 15:04
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.

4 participants