search

Midnighter / structurizr-python

A Python 3 port of Simon Brown's Structurizr diagrams-as-code architecture description tool.
https://structurizr.com/
Apache License 2.0
65 stars 16 forks source link

Improve syntax for the client locking a workspace as a context manager #62

Closed yt-ms closed 3 years ago

yt-ms commented 3 years ago

Checklist

Is your feature related to a problem? Please describe it.

Currently, you can lock a workspace with syntax like this:

    client = StructurizrClient(settings=settings)
    with client:
        client.put_workspace(workspace)

This is cool, but a little unintuitive that it's actually taking a lock out. I think it would be better to be more explicit about what's going on:

    client = StructurizrClient(settings=settings)
    with client.lock():
        client.put_workspace(workspace)

I can't decide whether it is better to have lock as a method or a property, but am leaning toward method as shown.

Midnighter commented 3 years ago

I agree with it being a method. To me that's much clearer because you are performing an action - locking the workspace - and not accessing a property. I'm now wondering if it should be .lock() or .lock_workspace()...

yt-ms commented 3 years ago

Well - .lock_workspace() would be consistent with get_workspace() and put_workspace(), but it already exists. Not sure whether we want to make it so the only way to lock/unlock is through a context manager.

Midnighter commented 3 years ago

No, you're right. It's better if those methods continue to exist separately.