Opentelemetry-java: Do not merge non-trivial changes during weekends

[carlosalberto]

This weekend a few non-trivial, non bug fixing PRs have been merged (https://github.com/open-telemetry/opentelemetry-java/pull/1945, which basically makes us non Specification-compliant, but ok).

We had already discussed by January, if memory serves me correctly. My problem with this is:

1) I don't want to be _forced_ to pay attention to PRs during the weekend, and reverting them or similar during Monday is not a good solution, IMHO.
2) It's easy that such changes get lost and I lose track of them amidst the start of next week.

I think it's not hard to wait for Monday, EOD Pacific, to merge PRs created on late Friday or over the weekend. For the record: this was extensively discussed (and enough complains were filled about me merging too fast or on Fridays PRs) in the Specification, for example. So let's keep it simple and organized.

[Oberon00]

which basically makes us non Specification-compliant

Please explain (best in a different issue)

[bogdandrutu]

Roger that for the merging part, happy to revert it if that helps.

Please consider to explain why is that not compliant with the specs, you mentioned that in a separate PR, and I tried to explain that it does not make things incompatible.

[carlosalberto]

From https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/trace/api.md#context-interaction

they MAY be exposed as static methods on the trace module, as static methods on a class inside the trace module (it MAY be named TracingContextUtilities), or on the Tracer class.

Although this is a MAY, I'd expect we'd try to put (or at least leave) them in, based on this text:

• Under io.opentelemetry.trace (not possible in Java though)
• A dedicated class such as TracingContextUtilities.
• In the Tracer class.

I'm fine with having it _anywhere_ in the API, but as we explicitly put the TracingContextUtils example for the Java case, I'd expect we keep things there (I had -mis-understood the methods in Span were simple shortcuts, for example). If that's not the case, lets amend the Specification to say this functionality must exist anywhere in the API, and that's all 😉

[Oberon00]

as static methods on a class inside the trace module

I think this is what we are doing

[bogdandrutu]

If that's not the case, lets amend the Specification to say this functionality must exist anywhere in the API, and that's all 😉

Happy to do that if that helps, as long as we keep all the other requirements (fully implemented, not duplicated, etc.).

[carlosalberto]

@Oberon00 @bogdandrutu I think I will propose removing the mention of TracingContextUtils in this part of the Specification, making the case very general. Fully implemented and not duplicated can definitely stay ;)

For this specific issue, I will simply open a PR mentioning in the docs we DONT recommend non-trivial, non bug-fixing PRs, if that's ok.

[bogdandrutu]

@carlosalberto please do that in the contribution.md, and sorry again for the trouble.

PS: Looks like java maintainers work all the time :)

[anuraaga]

I don't know if weekend should be special cased here - while many of us are employed, I think it's pretty normal for OSS to continue in weekends. I often joke the biggest benefit for an employer to do more OSS is to get free work on weekends :) That's not to say everyone must do so, but I wouldn't put restrictions on weekend work.

The other reason not to treat weekend specially is that the same could happen on business days - for the same reason, since this is OSS there may be weekdays where someone doesn't get a chance to check the repo since they're doing something else, so it seems to be the same situation.

The called-out PR had a maintainer author it and two other maintainers approve it, making no real change to public API. This should be enough to merge even on a weekend IMO. We can always follow up if there are issues.