Skip to content

Conversation

@cableman
Copy link
Contributor

@cableman cableman commented Apr 25, 2024

Link to ticket

https://leantime.itkdev.dk/?tab=timesheet#/tickets/showTicket/1064

Description

Audit module to log audit message to configurable backed using Drupal's plugin API.

Screenshot of the result

Screenshot from 2024-04-26 13-13-54

Checklist

  • My code passes our static analysis suite.
  • My code passes our continuous integration process.

If your code does not pass all the requirements on the checklist you have to add a comment explaining why this change
should be exempt from the list.

Additional comments or questions

N/A

@cableman cableman force-pushed the develop branch 12 times, most recently from 714da60 to f1a4c8e Compare April 26, 2024 08:12
@cableman cableman force-pushed the develop branch 8 times, most recently from 3a51f97 to 11d06f0 Compare April 26, 2024 11:13
Copy link
Contributor

@jekuaitk jekuaitk left a comment

Choose a reason for hiding this comment

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

Generally looks good!

I think we should consider renaming the $line parameter to $message.

* Name of the plugin that started the exception.
*
* @return string
* Name of the plugin if given else "Unknown plugin".
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just NULL?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because NULL is not human friendly as most exception will to send to system logs

foreach ($curlOptions as $option) {
[$key] = explode(' =>', $option);
$key = trim($key);
if (!defined($key)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe im not fully understanding this. Will this not accept any PHP constants? Is it just to filter out some invalid options?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is should check that it is one of: https://www.php.net/manual/en/function.curl-setopt.php

Have tried to update this part.

cableman and others added 9 commits May 3, 2024 13:44
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
Co-authored-by: Jeppe Kuhlmann Andersen <78410897+jekuaitk@users.noreply.github.com>
@cableman cableman requested a review from jekuaitk May 3, 2024 12:13
@cableman cableman merged commit 203d215 into main May 6, 2024
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.

2 participants