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

BEP for ORM implementation of Python driver (BEP-11) #57

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

innopreneur
Copy link
Contributor

BEP explaining motivation and implementation of python orm driver

@ttmc ttmc changed the title BEP for Orm implementation of python driver BEP for Orm implementation of python driver (BEP-21) Jul 9, 2018
@ttmc
Copy link
Contributor

ttmc commented Jul 10, 2018

It seems BEP-21 is now taken. You can probably use the name BEP-11 instead because nobody else seems to be using that number. When you make the change, please update the title of this pull request as well.

21/README.md Outdated
@@ -0,0 +1,60 @@
```
shortname: 21/PYTHON-ORM-DRIVER
name: Orm implementation for official bigchaindb python driver
Copy link
Contributor

Choose a reason for hiding this comment

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

Please write the name in title case: ORM Implementation for Official BigchainDB Python Driver

21/README.md Outdated

# Specification

This Orm should be implemented on top of python driver - [https://github.com/bigchaindb/bigchaindb-driver](https://github.com/bigchaindb/bigchaindb-driver).
Copy link
Contributor

Choose a reason for hiding this comment

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

Please write ORM in all caps, here and elsewhere. It's an acronym.

21/README.md Outdated
This Orm should be implemented on top of python driver - [https://github.com/bigchaindb/bigchaindb-driver](https://github.com/bigchaindb/bigchaindb-driver).
This implementation SHOULD include following **CRAB** operations -

1. **C**reate - performs CREATE transaction with asset payload into bigchainDB.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a specification for a Python ORM, it should include specific names and signatures (argument lists) for the functions, classes, methods or whatever must be implemented. The names and signatures could be inspired by the ones used in the JavaScript ORM, but of course names should be "Pythonic."

https://www.python.org/dev/peps/pep-0008/#naming-conventions

@ttmc
Copy link
Contributor

ttmc commented Jul 10, 2018

Also, please add a line for this BEP in the root README.md file of this repository (in the table).

@innopreneur innopreneur changed the title BEP for Orm implementation of python driver (BEP-21) BEP for Orm implementation of python driver (BEP-11) Jul 13, 2018
11/README.md Outdated
1. **C**reate - performs CREATE transaction with asset payload into bigchainDB.

```
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

These are C-style comments, but this is a specification for Python functions or methods, so they really should be Python-style comments, no? You can tell GitHub to format the code as Python by putting

```python

at the top of the code block. The comments could be Python docstrings, i.e. after the def line.

11/README.md Outdated
* creates the asset in bigchaindb
* @param {dict} inputs - dictionary containing keypairs and data for the asset
*/
def create(inputs)
Copy link
Contributor

Choose a reason for hiding this comment

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

I find it very strange that each function/method has exactly one input and it's always a dict. Why not a list of arguments and/or keyword arguments?

@@ -0,0 +1,93 @@
```
shortname: 11/PYTHON-ORM-DRIVER
Copy link
Contributor

Choose a reason for hiding this comment

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

If you change the shortname of this BEP to BEP-11 then we can close issue #32 because all other shortnames have been updated to the BEP-N format.

@ttmc ttmc added the new BEP label Aug 7, 2018

# Copyright Waiver

_To the extent possible under law, the person who associated CC0 with this work has waived all copyright and related or neighboring rights to this work._
Copy link
Contributor

Choose a reason for hiding this comment

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

Please replace the current copyright waiver text with the following block of HTML. (HTML is valid Markdown.)

<p xmlns:dct="http://purl.org/dc/terms/">
  <a rel="license"
     href="http://creativecommons.org/publicdomain/zero/1.0/">
    <img src="http://i.creativecommons.org/p/zero/1.0/88x31.png" style="border-style: none;" alt="CC0" />
  </a>
  <br />
  To the extent possible under law, all contributors to this BEP
  have waived all copyright and related or neighboring rights to this BEP.
</p>

@ttmc ttmc changed the title BEP for Orm implementation of python driver (BEP-11) BEP for ORM implementation of Python driver (BEP-11) Sep 13, 2018
@ttmc
Copy link
Contributor

ttmc commented Sep 14, 2018

I talked to @innoprenuer about this PR today. He said that some folks on the Ocean team already wrote a BigchainDB ORM in Python, but we haven't gotten around to open sourcing it yet because other things have been higher priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants