On 14 October 2016 at 17:12, Shahim Essaid <[email protected]> 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 [email protected]. > 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
