On 14 October 2016 at 17:12, Shahim Essaid <sha...@essaid.com> wrote:

> Hi,
>

Hi Shahim,

What is the best way to ask questions if I'm reviewing some code and I
> think I see a problem, or I need some clarification? I know GitHub has a
> commenting feature for pull requests and on blame views but there is no way
> to simply place comments on arbitrary lines in current files.  I also
> looked for an issue label like "code review" or something similar and I
> didn't notice one.
>
> Examples of what I'm talking about are:
>
> 1. Why is there no checkSecurity here:
>
> https://github.com/orientechnologies/orientdb/blob/
> b431805894b2599c3c7b4f066d31800ca504b389/core/src/main/java/
> com/orientechnologies/orient/core/db/document/
> ODatabaseDocumentTx.java#L1348
>
> as seen for the similar method here:
>
> https://github.com/orientechnologies/orientdb/blob/
> b431805894b2599c3c7b4f066d31800ca504b389/core/src/main/java/
> com/orientechnologies/orient/core/db/document/
> ODatabaseDocumentTx.java#L1365
>

You're right it's missing.


> 2. Database listeners are sometimes called for onBeforeTxBegin before the
> actual transaction object is set for the database. Is this intentional?
> Examples:
>
> https://github.com/orientechnologies/orientdb/blob/
> b431805894b2599c3c7b4f066d31800ca504b389/core/src/main/java/
> com/orientechnologies/orient/core/db/document/
> ODatabaseDocumentTx.java#L1736
>
> https://github.com/orientechnologies/orientdb/blob/
> b431805894b2599c3c7b4f066d31800ca504b389/core/src/main/java/
> com/orientechnologies/orient/core/db/document/
> ODatabaseDocumentTx.java#L2284
>

It's intentional, also the method's name onBeforeTxBegin() tells it's
called before the transaction begins.


> 3. DRY
>
> https://github.com/orientechnologies/orientdb/blob/
> b431805894b2599c3c7b4f066d31800ca504b389/core/src/main/java/
> com/orientechnologies/orient/core/db/document/
> ODatabaseDocumentTx.java#L2341
>
> can simply be  this.freeze(false);
>
> I'm starting to study some of the code and I'm sure I'll have many similar
> questions. Should these be issues? One issue per file? Which label should I
> use?
>

You're right, it could call that methof.

WDYT about sending a Pull Request on GitHub against the develop branch? I'd
be more than happy to accept it.


Best Regards,

Luca Garulli
Founder & CEO
OrientDB LTD <http://orientdb.com/>

Want to share your opinion about OrientDB?
Rate & review us at Gartner's Software Review
<https://www.gartner.com/reviews/survey/home>



>
> Best,
> Shahim
>
> --
>
> ---
> You received this message because you are subscribed to the Google Groups
> "OrientDB" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to orient-database+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 

--- 
You received this message because you are subscribed to the Google Groups 
"OrientDB" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to orient-database+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to