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

Get external dependencies out of source tree #5

Open
abendebury opened this issue Dec 11, 2015 · 15 comments
Open

Get external dependencies out of source tree #5

abendebury opened this issue Dec 11, 2015 · 15 comments
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: patch A pull request is required Priority: high High priority
Milestone

Comments

@abendebury
Copy link

abendebury commented Dec 11, 2015

e.g. musicbrainz

@abendebury abendebury modified the milestone: 2.0 Dec 13, 2015
@JoeLametta
Copy link
Collaborator

JoeLametta commented Jun 16, 2016

MusicBrainz was migrated long time ago.

Still remaining:

@tobbez
Copy link
Contributor

tobbez commented Sep 3, 2016

Click might be a good fit for replacing python-command (unless someone wants to package and support python-command instead).

flog could possibly be replaced with Python's builtin logging facilities.

@JoeLametta
Copy link
Collaborator

Thanks for the suggestions (didn't know about Click).

flog could possibly be replaced with Python's builtin logging facilities.

Yeah that should suffice.

@MerlijnWajer
Copy link
Collaborator

On 07/09/16 18:19, JoeLametta wrote:

Thanks for the suggestions (didn't know about Click
http://click.pocoo.org/).

flog could possibly be replaced with Python's builtin logging
facilities.

Yeah that should suffice.

I have made a simply stub that calls python's logging module in the
internal functions of 'flog'. It works fine, but it's not the most clean
way to do it.

@MerlijnWajer
Copy link
Collaborator

MerlijnWajer commented Sep 8, 2016

The proper way to get rid of flog would be to replace all the logging calls with the normal python ones. I would suggest to also look at how logging should be used from a library. We/I use morituri (soon whipper) as a library, and it is very nice if the libraries respect the proper logging procedures.

The fix I mentioned above was to make moritur1 behave properly wrt logging. It doesn't get rid of the flog dependency.

@JoeLametta
Copy link
Collaborator

The proper way to get rid of flog would be to replace all the logging calls with the normal python ones. I would suggest to also look at how logging should be used from a library. We/I use morituri (soon whipper) as a library, and it is very nice if the libraries respect the proper logging procedures.

The fix I mentioned above was to make moritur1 behave properly wrt logging. It doesn't get rid of the flog dependency.

All right I've no real experience providing logging for a library: could you give me some examples regarding this one?

Thanks

@MerlijnWajer
Copy link
Collaborator

http://docs.python-guide.org/en/latest/writing/logging/
http://pythonsweetness.tumblr.com/post/67394619015/use-of-logging-package-from-within-a-library

These seem useful. I can try to do it 'properly' for the logging calls, and likely in an automated manner replace most of the logging calls. That still leaves us with all the 'log.Loggable' things, which is inherited from quite some classes, IIRC.

@MerlijnWajer
Copy link
Collaborator

As for python-command, I would opt to simply replace it with argparse... But that'll require rewriting some code.

@RecursiveForest RecursiveForest self-assigned this Oct 21, 2016
@RecursiveForest
Copy link
Contributor

I also opt to simply rewrite the python-command stuff with argparse. Click seems like potentially a nice library, but the specific use case here and the amount of changes needed to just move to argparse are pretty small. I will do this myself since no one seems to have decided to own it.

Similarly: Merlijn, do you want to "officially" take the logging replacement work? In terms of prioritisiation I'd rather see -gst > this, so I'm willing to take it if you don't want it, but it's a low enough priority that I'm fine waiting for both.

@RecursiveForest
Copy link
Contributor

I originally came here to mention that there are more bundled external libraries than just those that are fetched with git submodules. Namely asyncsub and task. I haven't given either more than a cursory look, but are we wanting to remove them as well?

@JoeLametta
Copy link
Collaborator

I've just given it a quick look but are we sure asyncsub is still needed with Python 2.7 subprocess available?
About task I don't know either: there's gstreamer support code bundled which is probably going to be deleted when finalizing GSt removal.

@RecursiveForest
Copy link
Contributor

task seems dependent on gobject and I don't think the -gst patch Merlijn has in the works is going to cover that. I think between subprocess and threading/multiprocessing we can remove gobject and asyncsub, but it will be no small undertaking.

@RecursiveForest
Copy link
Contributor

RecursiveForest commented Dec 18, 2016

#92 removes python-command and flog.

@ArchangeGabriel
Copy link

And it’s now merged, so asyncsub/gobject/task are the new remaining ones.

@JoeLametta
Copy link
Collaborator

JoeLametta commented Dec 21, 2016

Remaining (as of 2016/12/21):

@Freso Freso mentioned this issue Apr 28, 2017
4 tasks
@JoeLametta JoeLametta added Bug Generic bug: can be used together with more specific labels Accepted Accepted issue on our roadmap Priority: high High priority Needed: patch A pull request is required and removed enhancement labels Nov 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug Generic bug: can be used together with more specific labels Needed: patch A pull request is required Priority: high High priority
Projects
None yet
Development

No branches or pull requests

6 participants