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

Sprint 002 issue 150 reup #210

Merged
merged 6 commits into from
May 12, 2016
Merged

Sprint 002 issue 150 reup #210

merged 6 commits into from
May 12, 2016

Conversation

br2490
Copy link

@br2490 br2490 commented Apr 26, 2016

Clean pull request.
Continues work on #150 after resolving issues with Windows Vagrants...

Also note that the PSR2 thingy I use removed, which it probably shouldn't have.
use Symfony\Component\HttpFoundation\Response;

@ruebot @whikloj @DiegoPino This PR is better than my last.

@ruebot
Copy link
Member

ruebot commented Apr 26, 2016

@br2490 does this supersede #206?

@ruebot ruebot added this to the Community Sprint - 06 milestone Apr 26, 2016
@br2490
Copy link
Author

br2490 commented Apr 26, 2016

Yes.

Sent from my phone. Forgive brevity and autocorrect.
On Apr 26, 2016 6:57 PM, "Nick Ruest" notifications@github.com wrote:

@br2490 https://github.com/br2490 does this supersede #206
#206?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#210 (comment)

@ruebot
Copy link
Member

ruebot commented May 11, 2016

@whikloj @DiegoPino should we review this and #169, and get them merged before we split everything apart?

@whikloj
Copy link
Member

whikloj commented May 11, 2016

I'll test this, but I think the issue was when @DiegoPino and I spoke about this. We figured this should probably go into the (yet to be created) PcdmObjectService. That way we leave the core delete functionality alone.

If it works, I'll see if I can move it via a PR on @br2490's PR.

Removed debug (error_log) lines; still need to determine an appropriate response && wrap in a TX if !TX. Someone with more exp., please? Also I'm on vacation enjoying life. <3
@whikloj
Copy link
Member

whikloj commented May 12, 2016

Actually, there is no way to commit this to a new file without losing the contributions. So I'll test it tomorrow and if it works, we can merge it and then create a new task to create the PcdmObjectService and move this delete functionality there instead of leaving it here.

But @br2490, it looks like you have merge conflicts. Can you rebase this on origin/sprint-002?

@br2490
Copy link
Author

br2490 commented May 12, 2016

@whikloj assuming I've managed the merge + Travis likes my code compliance, I think we're okay.

@br2490
Copy link
Author

br2490 commented May 12, 2016

Dear Travis,

It has come to my attention you're aware that I lint files like I clean my lint trap; infrequently and poorly.
Please accept my current commit as recompense for my haphazardness.

Best,
Ben - still totally on vacation.

@ruebot
Copy link
Member

ruebot commented May 12, 2016

@br2490 you can blame me for getting coding standards setup! ...and you can enjoy your vacation, and have me get it up to standards if you want. I can do a pull against your pull if it fails again.

@br2490
Copy link
Author

br2490 commented May 12, 2016

@ruebot I've got this, it's just two lines that are too long. ;)

@whikloj whikloj merged commit 842e6af into Islandora:sprint-002 May 12, 2016
@br2490 br2490 deleted the sprint-002-issue-150-reup branch May 12, 2016 17:41
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.

3 participants