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

newbot: Add --log option to newbot CLI. #78

Merged
merged 2 commits into from
Sep 30, 2022

Conversation

locomotiveviaduct
Copy link
Contributor

@locomotiveviaduct locomotiveviaduct commented Sep 26, 2022

Signed-off-by: Mark A mkand@microsoft.com

resolves #80

Signed-off-by: Mark A <mkand@microsoft.com>
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Hi, thanks a lot for the PR.

When I added newbot, I intentionally did not add back the --log argument as my plan was/is to replace the logging system with a new one. The existing logging implementation is quite bad for what people need it to do, it is usually better to just "log" the stdout from newbot as it contains more valuable information in a lot of cases.

If your usecase is test-result reporting, I would actually switch away from the newbot interface entirely and go with pytest instead as detailed here: pytest Integration

That said, I can see how some people would like to still access the existing logging through newbot anyway. My suggestion is that we name the CLI flag something else so --log stays free for whatever comes to replace the current logging.

What about --legacy-log? Or if that suggests too much that a new implementation already exists, what about --json-log-stream?


Beyond this, a slightly different topic: I am thinking about relicensing (parts of) tbot in the near future. You probably know how painful this can get for open-source projects... So it may be better to ask upfront: Would it be possible for you to give me written permission to relicense your contribution(s) under one or more of the OSI approved licenses in the future?

@locomotiveviaduct
Copy link
Contributor Author

Thanks for the response.

We use --log in our existing tbot invocations, so it would be helpful to minimize the disruption in migrating to newbot. Renaming the option to something like --legacy-log or --json-log-stream sounds reasonable.

Can I get back to you on the license part of your question?

@Rahix
Copy link
Owner

Rahix commented Sep 28, 2022

We use --log in our existing tbot invocations, so it would be helpful to minimize the disruption in migrating to newbot.

Yeah, this is what I was thinking.

Can I get back to you on the license part of your question?

Sure, sorry for this...

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Code looks good, I will wait for your reply on the other topic :)

@locomotiveviaduct
Copy link
Contributor Author

I've spoken to our legal folks, and you have permission to sublicense the contribution under GPLv3 later under an OSI license of your choice.

Thanks for waiting.

@Rahix
Copy link
Owner

Rahix commented Sep 30, 2022

Alright, thank you very much!

@Rahix Rahix merged commit c743375 into Rahix:master Sep 30, 2022
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.

newbot CLI cannot generate log files
2 participants