konflux-ci / architecture

Technical and Architecture documents
https://konflux-ci.dev/architecture/
18 stars 70 forks source link

docs(RHTAPWATCH-314): Add Splunk-related logging documentation #117

Closed eisraeli closed 1 year ago

eisraeli commented 1 year ago

Added new Logging documentation.

Included general information about Open Shift Logs, Splunk and logs retention in Splunk.

Omeramsc commented 1 year ago

I'd expect the details you mentioned on Slack to be available on the commit description section...

eisraeli commented 1 year ago

I'd expect the details you mentioned on Slack to be available on the commit description section...

I don't think that information is relevant for the description.

Omeramsc commented 1 year ago

I'd expect the details you mentioned on Slack to be available on the commit description section...

I don't think that information is relevant for the description.

I don't agree. Since when do we shy away from mentioning what or why we made a change? It was mentioned that most of this change was just moving documentation from one place to another - how is that not relevant? it means it is not new and we already approved most of the "code" (documentation) here before.

eisraeli commented 1 year ago

@Omeramsc I don't agree. Since when do we shy away from mentioning what or why we made a change? It was mentioned that most of this change was just moving documentation from one place to another - how is that not > relevant? it means it is not new and we already approved most of the "code" (documentation) here before.

It was a submitted as PR to the SOP repo, but was never merged there, but it was suggested to move it to this repo. So it's not like we actually moved anything that was already merged from that repo to this repo.

Omeramsc commented 1 year ago

@Omeramsc I don't agree. Since when do we shy away from mentioning what or why we made a change? It was mentioned that most of this change was just moving documentation from one place to another - how is that not > relevant? it means it is not new and we already approved most of the "code" (documentation) here before.

It was a submitted as PR to the SOP repo, but was never merged there, but it was suggested to move it to this repo. So it's not like we actually moved anything that was already merged from that repo to this repo.

I still don't understand how that makes adding a bit of context to the commit message bad...

eisraeli commented 1 year ago

@Omeramsc I don't agree. Since when do we shy away from mentioning what or why we made a change? It was mentioned that most of this change was just moving documentation from one place to another - how is that not > relevant? it means it is not new and we already approved most of the "code" (documentation) here before.

It was a submitted as PR to the SOP repo, but was never merged there, but it was suggested to move it to this repo. So it's not like we actually moved anything that was already merged from that repo to this repo.

I still don't understand how that makes adding a bit of context to the commit message bad...

Nothing bad about it. I've added a description.

eisraeli commented 1 year ago

@Omeramsc Any other feedback?