Logger name standards?

[Tom Morris]

I was going to take a crack getting rid of some of the many Throwable.printStackTrace() calls that we have scattered about, converting them to use SLF4J logging, but in the process noticed that we've got a diversity of naming schemes for the loggers which include (complete list in the attachment):

1. lowercase snake_case based loosely on class name or functional area e.g. "refine_server"

2. as above with spaces e.g. "open office" for OdsImporter

3. as above with hyphens e.g. "refine-client"

4. a mix of separators e.g. "importing-controller_command"

5. Java class name in CamelCase using both string ("JsonParser") and static class (JsonImporter.class) as well as instance variable (getClass())

I don't think it's a high priority to fix the existing usage, but it would be useful to have a standard naming scheme going forward.

I propose that static loggers be declared using:

static final private Logger logger = LoggerFactory.getLogger(MyClassName.class);

and that instance loggers using:

final protected Logger logger = LoggerFactory.getLogger(getClass());

When we have agreement, I can add it to the vestigial https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards 

Tom

[ow...@ostephens.com]

Having a convention for this would be a good idea and the proposal looks good to me - although I admit to not having any great insight into this, but it seems a sensible convention and tbh any consistent convention seems like it would be better than the variety we currently have.

So consider this a +1 from me

Owen

[Antonin Delpeuch (lists)]

Hi Tom,

Thanks for suggesting this and sorry about the slow reaction! What you
propose looks sensible, let's go for this.

Best,
Antonin

> https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards <https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards>
>
> Tom
>
> --
> You received this message because you are subscribed to the Google
> Groups "OpenRefine Development" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to openrefine-de...@googlegroups.com
> <mailto:openrefine-de...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com <https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com?utm_medium=email&utm_source=footer>.

[Tom Morris]

I've updated the wiki page. When doing so I noticed that it said people could use "Draft" Pull Requests which I was told during a recent code review are strongly discouraged, so perhaps that should be updated (the logging standard update was the first since the page was written in Mar 2020).

Tom

To unsubscribe from this group and stop receiving emails from it, send an email to openrefine-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/openrefine-dev/c8a0dfda-69af-a528-a35c-d67be27c2283%40antonin.delpeuch.eu.

[Antonin Delpeuch (lists)]

Hi Tom,

There is a misunderstanding here: I never said that draft PRs are
strongly discouraged.

I just think it is okay to comment on a PR whatever its stage is,
because it is a publicly visible item on the list of PRs opened against
the repository.

To run the CI on a set of changes privately, one could recommend people
to open PRs against the master branch of their own GitHub fork. I think
that should run the CI, and the PR would appear in the list of PRs of
their own fork, not the official repository. And I would not get
notified about it and would therefore not comment.

Hopefully GitHub will improve their draft PR feature to fix this:
https://github.com/community/community/discussions/10409

Best
Antonin

On 25/11/2022 04:31, Tom Morris wrote:
> I've updated the wiki page. When doing so I noticed that it said people
> could use "Draft" Pull Requests which I was told during a recent code
> review are strongly discouraged, so perhaps that should be updated (the
> logging standard update was the first since the page was written in Mar
> 2020).
>
> Tom
>
> On Thu, Nov 3, 2022 at 9:50 AM Antonin Delpeuch (lists)
> <li...@antonin.delpeuch.eu <mailto:li...@antonin.delpeuch.eu>> wrote:
>
> Hi Tom,
>
> Thanks for suggesting this and sorry about the slow reaction! What you
> propose looks sensible, let's go for this.
>
> Best,
> Antonin
>
> On 25/10/2022 10:27, ow...@ostephens.com

> https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards <https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards> <https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards <https://github.com/OpenRefine/OpenRefine/wiki/Development-coding-standards>>

> >
> >     Tom
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "OpenRefine Development" group.
> > To unsubscribe from this group and stop receiving emails from it,
> send
> > an email to openrefine-de...@googlegroups.com

> <mailto:openrefine-dev%2Bunsu...@googlegroups.com>
> > <mailto:openrefine-de...@googlegroups.com
> <mailto:openrefine-dev%2Bunsu...@googlegroups.com>>.

> > To view this discussion on the web visit
> >

> https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com <https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com> <https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com?utm_medium=email&utm_source=footer <https://groups.google.com/d/msgid/openrefine-dev/63cd6154-9c47-402d-b5ae-9f29c6b8b987n%40googlegroups.com?utm_medium=email&utm_source=footer>>.

>
> --
> You received this message because you are subscribed to the Google
> Groups "OpenRefine Development" group.
> To unsubscribe from this group and stop receiving emails from it,
> send an email to openrefine-de...@googlegroups.com

> <mailto:openrefine-dev%2Bunsu...@googlegroups.com>.

> To view this discussion on the web visit

> https://groups.google.com/d/msgid/openrefine-dev/c8a0dfda-69af-a528-a35c-d67be27c2283%40antonin.delpeuch.eu <https://groups.google.com/d/msgid/openrefine-dev/c8a0dfda-69af-a528-a35c-d67be27c2283%40antonin.delpeuch.eu>.

>
> --
> You received this message because you are subscribed to the Google
> Groups "OpenRefine Development" group.
> To unsubscribe from this group and stop receiving emails from it, send

> an email to openrefine-de...@googlegroups.com
> <mailto:openrefine-de...@googlegroups.com>.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/openrefine-dev/CAE9vqEEDJbS%2B8kkWaecGN0c7Z-AOKDObpvQsHiuJdLSybxk1JA%40mail.gmail.com <https://groups.google.com/d/msgid/openrefine-dev/CAE9vqEEDJbS%2B8kkWaecGN0c7Z-AOKDObpvQsHiuJdLSybxk1JA%40mail.gmail.com?utm_medium=email&utm_source=footer>.