Re: [PATCH v2 01/12] simpletrace: Improve parsing of sys.argv; fix files never closed.

[Stefan Hajnoczi]

On Mon, May 08, 2023 at 01:18:40PM +0000, Mads Ynddal wrote:
> > simpletrace.py is both a command-line tool and a Python module. The
> > Python module has a public API that people's scripts may rely on. Let's
> > avoid breaking API changes unless necessary so that existing scripts
> > that import simpletrace continue to work.
> > 
> > It's not very clear what is a public API in simpletrace.py, the file
> > should really have __all__ = ['Analyzer', 'process', 'run'].
> > Nevertheless, Analyzer's doc comments mention process() and the
> > process() function itself also has doc comments, so it's a public API.
> > 
> > Please drop this change to avoid breaking the public API.
> 
> I agree, I'll revert the changes. I can add an `__all__` too.
> 
> I'd like to avoid having the same `open`, `read_trace_header` and 
> `read_events`
> multiple places. Would it be acceptable to let `process` be more of a stub and
> move the logic to an internal `_process` function maybe?

Yes, as long as the existing public API doesn't change that would be
fine.

Stefan

signature.asc
Description: PGP signature