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

I have added CTI support in this fork #139

Closed
wants to merge 9 commits into from

Conversation

vasil-yordanov
Copy link

No description provided.

if (!$this->metadataFactory->isAudited($className)) {
throw new NotAuditedException($className);
throw AuditException::notAudited($className);
Copy link
Contributor

Choose a reason for hiding this comment

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

@vasil-yordanov You may have forgotten to add the AuditException file.

Copy link
Author

Choose a reason for hiding this comment

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

Hi,

Isn't this the same as

new NotAuditedException($className);

?

Have you merged my branch to master?

Regards,

Vasil.

On Fri, Oct 9, 2015 at 8:25 PM, Simon Mönch [email protected]
wrote:

In src/SimpleThings/EntityAudit/AuditReader.php
#139 (comment)
:

     if (!$this->metadataFactory->isAudited($className)) {
  •        throw new NotAuditedException($className);
    
  •        throw AuditException::notAudited($className);
    

@vasil-yordanov https://github.com/vasil-yordanov You may have
forgotten to add the AuditException file.


Reply to this email directly or view it on GitHub
https://github.com/simplethings/EntityAudit/pull/139/files#r41655545.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, NotAuditedException isn't the same name as AuditException 😉 Do you forget a use statement or adding the class?

The builds are failed, see https://travis-ci.org/simplethings/EntityAudit/builds/69906870

PHP Fatal error:  Class 'SimpleThings\EntityAudit\AuditException' not found in /home/travis/build/simplethings/EntityAudit/src/SimpleThings/EntityAudit/AuditReader.php on line 411

Copy link
Author

Choose a reason for hiding this comment

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

ok I will fix this tomorrow.

Regards,

Vasil

On Fri, Oct 16, 2015 at 6:37 PM, David Badura [email protected]
wrote:

In src/SimpleThings/EntityAudit/AuditReader.php
#139 (comment)
:

     if (!$this->metadataFactory->isAudited($className)) {
  •        throw new NotAuditedException($className);
    
  •        throw AuditException::notAudited($className);
    

No, NotAuditedException isn't the same name as AuditException [image:
😉] Do you forget a use statement or add the class?

The builds are failed, see
https://travis-ci.org/simplethings/EntityAudit/builds/69906870

PHP Fatal error: Class 'SimpleThings\EntityAudit\AuditException' not found in /home/travis/build/simplethings/EntityAudit/src/SimpleThings/EntityAudit/AuditReader.php on line 411


Reply to this email directly or view it on GitHub
https://github.com/simplethings/EntityAudit/pull/139/files#r42256206.

Copy link
Author

Choose a reason for hiding this comment

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

Hello, I'm very sorry for my late response.
I fixed this issue.
Please let me know if there is still some chance to include my changes in
your code.

Regards,

Vasil.

On Fri, Oct 16, 2015 at 10:18 PM, Vasil Yordanov [email protected]
wrote:

ok I will fix this tomorrow.

Regards,

Vasil

On Fri, Oct 16, 2015 at 6:37 PM, David Badura [email protected]
wrote:

In src/SimpleThings/EntityAudit/AuditReader.php
#139 (comment)
:

     if (!$this->metadataFactory->isAudited($className)) {
  •        throw new NotAuditedException($className);
    
  •        throw AuditException::notAudited($className);
    

No, NotAuditedException isn't the same name as AuditException [image:
😉] Do you forget a use statement or add the class?

The builds are failed, see
https://travis-ci.org/simplethings/EntityAudit/builds/69906870

PHP Fatal error: Class 'SimpleThings\EntityAudit\AuditException' not found in /home/travis/build/simplethings/EntityAudit/src/SimpleThings/EntityAudit/AuditReader.php on line 411


Reply to this email directly or view it on GitHub
https://github.com/simplethings/EntityAudit/pull/139/files#r42256206.

DavidBadura and others added 2 commits October 16, 2015 17:26
No, NotAuditedException isn't the same name as AuditException 😉 Do you forget a use statement or add the class?

The builds are failed, see https://travis-ci.org/simplethings/EntityAudit/builds/69906870

PHP Fatal error:  Class 'SimpleThings\EntityAudit\AuditException' not found in /home/travis/build/simplethings/EntityAudit/src/SimpleThings/EntityAudit/AuditReader.php on line 411
@DavidBadura
Copy link
Contributor

it has merge conflicts - can you it rebase please?

@vasil-yordanov
Copy link
Author

Ok, I will do it now

On Tue, Nov 10, 2015 at 1:51 PM, David Badura [email protected]
wrote:

it has merge conflicts - can you it rebase please?


Reply to this email directly or view it on GitHub
#139 (comment)
.

@vasil-yordanov
Copy link
Author

Hello David,

It seems this is not a trivial merge I will need more time to do that, will
be not able to do the merge and the testing today. I hope will manage in
2-3 days to do that.

Regards,

Vasil.

On Tue, Nov 10, 2015 at 1:57 PM, Vasil Yordanov [email protected]
wrote:

Ok, I will do it now

On Tue, Nov 10, 2015 at 1:51 PM, David Badura [email protected]
wrote:

it has merge conflicts - can you it rebase please?


Reply to this email directly or view it on GitHub
#139 (comment)
.

@OskarStark
Copy link
Member

great work 👍

@DavidBadura
Copy link
Contributor

no problem. i would like to merge it before #159. thank you!

@vasil-yordanov
Copy link
Author

Hello David,

I give up to do the merge, its too complex.
It is a pity because I did a lot of code refactoring in my version where I
managed to shorten the methods. Currently the code has very long methods.
See my old code:
https://github.com/vasil-yordanov/EntityAudit/blob/4b5a6a4b9f37c6276f85d57b96d2d86a9f0a93a5/src/SimpleThings/EntityAudit/AuditReader.php

It seems that currently the feature (excluding the code refactoring) that I
have added previously in my repo is already available in the master of
simplethings/EntityAudit

However in my latest commit I did the following:

  1. I have merged my code with the master of simplethings/EntityAudit repo.
  2. I added some functional tests for Joined table inheritance.
  3. I fixed some small bugs found during the tests
  4. I did very small code refactoring.

I will try some shiny day to continue with the refactoring of the
code, but please
accept now my pull request,
and update the readme that we this lib
supports Joined table inheritance.

Regards,

Vasil.

On Tue, Nov 10, 2015 at 3:33 PM, David Badura [email protected]
wrote:

no problem. i would like to merge it before #159
#159. thank you!


Reply to this email directly or view it on GitHub
#139 (comment)
.

@smoench
Copy link
Contributor

smoench commented Dec 30, 2015

@vasil-yordanov Could you please open a new clean PR with your latest changes? Thanks in advance

@smoench smoench closed this Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants