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.

Sign up for GitHub

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

Jump to bottom

Lightroom 5 support #95

Open
wants to merge 9 commits into
base: v2.6.x
Choose a base branch
from
Open

Lightroom 5 support #95

wants to merge 9 commits into from

Conversation

gloubibou
Copy link

No description provided.

Pierre Bernard added 4 commits July 8, 2013 11:31
@mikeabdullah
Added Lightroom 5 support
bde5aea 
The Lightroom 5 code was intially written for
github.com/iMediaSandboxing and adapted to fit the iMedia 2.6
architecture
@mikeabdullah
Query generation methods are now instance methods
a6dc418 
They check self.mediaType when creating the query SQL
@mikeabdullah
Added Lightroom 5 video parser
026f158
@mikeabdullah
Expose iMedia/IMBLightroomObject.h
6928d03
mikeabdullah
mikeabdullah reviewed Jul 8, 2013
View reviewed changes
IMBLightroomParser.m
#import "NSString+iMedia.h"
#import "NSWorkspace+iMedia.h"
#import "NSURL+iMedia.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hardly the end of the world, but doesn't this file #import NSURL+iMedia.h twice?

Copy link
Author

Choose a reason for hiding this comment

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

It does ;-)

On Jul 8, 2013, at 4:25 PM, Mike Abdullah [email protected] wrote:

In IMBLightroomParser.m:

#import "NSString+iMedia.h"
#import "NSWorkspace+iMedia.h"
+#import "NSURL+iMedia.h"
Hardly the end of the world, but doesn't this file #import NSURL+iMedia.h twice?

—
Reply to this email directly or view it on GitHub.

@thebenjiman
Copy link

hi, did few improv on the fs watcher for my job, you can take a look;
don't know how to pull on github.

Best Regards,
Benjamin Domergue

On Sun, Jul 7, 2013 at 11:37 PM, Pierre Bernard [email protected]:

You can merge this Pull Request by running

git pull https://github.com/gloubibou/iMedia cleanup

Or view, comment on, or merge it at:

#95
Commit Summary

  • Prevent crash on nil statement
  • Replaced NSInteger with int
  • Cast to keep the compiler happy
  • Release instance variables in owning class
  • Cast to keep the compiler happy
  • Print error if XML parsing fails
  • Naming conventions: keep analyzer happy
  • CLANG complained about possible nil key or value
  • Fixed error check
  • Casts to keep the compiler happy
  • Casts too keep the compiler happy
  • Use 10.7 NSRect conversion if available
  • Make sure not to mutate enumerated collection
  • Let the compiler know about a 10.7 only method
  • Check image source before use
  • GC compatibility
  • Fix cast to match return type
  • Added Lightroom 5 support
  • Query generation methods are now instance methods
  • Added Lightroom 5 video parser
  • Expose iMedia/IMBLightroomObject.h
  • NSURLBookmarkResolutionWithSecurityScope
  • mkstemp creates a file. We want a directory
  • NSURLBookmarkResolutionWithSecurityScope
  • Syntax not supported by Xcode 4.2
  • Keep compiler happy
  • pt_BR updates based on pull request 93 and consistency with Apple
    usage

File Changes

Patch Links:

@mikeabdullah
Copy link
Collaborator

@thebenjiman can you give us a link of some sort to your changes, please?

@thebenjiman
Copy link

I sent the svn patch in my first mail;
is there another email I can send the file to?

Best Regards,
Benjamin Domergue

On Mon, Jul 15, 2013 at 12:46 PM, Mike Abdullah [email protected]:

@thebenjiman https://github.com/thebenjiman can you give us a link of
some sort to your changes, please?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/95#issuecomment-20962385
.

@mikeabdullah
Copy link
Collaborator

Did you send it as an attachment with the email? If so, GitHub appears to ignore such things. If not, can you figure something else out, please? Ideally, opening a fresh issue for this would help, too.

@thebenjiman
Copy link

Use this archive:
https://www.dropbox.com/s/jk84jdfflalzvb0/finder-karelia-affected-files.zip
It has all affected file so you can do the diff directly, if some info
missing let me know

Best Regards,
Benjamin Domergue

On Mon, Jul 15, 2013 at 4:39 PM, TheBenjiman [email protected] wrote:

Some interesting ideas too here maybe....
https://www.dropbox.com/s/ssomiayh7j252y1/UKFSEventsWatcher.m

Best Regards,
Benjamin Domergue

On Mon, Jul 15, 2013 at 4:34 PM, TheBenjiman [email protected]:

Here it is,
https://www.dropbox.com/s/s04j30d2y7g7q7e/Karelia-finder-patch2.patch
No time to learn github sorry.

Best Regards,
Benjamin Domergue

On Mon, Jul 15, 2013 at 4:27 PM, Mike Abdullah [email protected]:

Did you send it as an attachment with the email? If so, GitHub appears
to ignore such things. If not, can you figure something else out, please?
Ideally, opening a fresh issue for this would help, too.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/95#issuecomment-20972921
.

@mikeabdullah
Copy link
Collaborator

Well I just downloaded the archive. It contains IMBFolderParser.m, IMBLibraryController.m, and a broken alias to UKFSEventsWatcher.m, so there's not a lot I can do with that.

Which revision/commit of iMedia are you making your changes from? If you're using SVN, I'm worried you might be on some extremely old version of the codebase.

@thebenjiman
Copy link

On Wed, Jul 17, 2013 at 11:40 AM, Mike Abdullah [email protected]:

UKFSEventsWatcher.m

Sorry about that, i fixed the archive
https://www.dropbox.com/s/jk84jdfflalzvb0/finder-karelia-affected-files.zip

Best Regards,
Benjamin Domergue

@mikeabdullah
Copy link
Collaborator

Wow, that's a pretty big batch of changes then. Which commit of iMedia are you working from? It certainly doesn't appear to be the latest. Also, what concrete "improvements" does this actually make?

@thebenjiman
Copy link

We are working with the 2.6.x branch;
We had a lot of inconsistencies with the folders node in the browser when
moving folders (present in iMedia finder node) to an expanded subnode from
the finder.
I seem to remember that we even had crashes sometimes. This patch will
prevent any inconsistency with the finder node. It will always be a
"mirror" of the finder represented folders.

Best Regards,
Benjamin Domergue

On Tue, Jul 30, 2013 at 1:50 PM, Mike Abdullah [email protected]:

Wow, that's a pretty big batch of changes then. Which commit of iMedia are
you working from? It certainly doesn't appear to be the latest. Also, what
concrete "improvements" does this actually make?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/95#issuecomment-21785460
.

@thebenjiman
Copy link

Karelia-iMedia-2.5.x sorry

Best Regards,
Benjamin Domergue

On Tue, Jul 30, 2013 at 2:58 PM, TheBenjiman [email protected] wrote:

We are working with the 2.6.x branch;
We had a lot of inconsistencies with the folders node in the browser when
moving folders (present in iMedia finder node) to an expanded subnode from
the finder.
I seem to remember that we even had crashes sometimes. This patch will
prevent any inconsistency with the finder node. It will always be a
"mirror" of the finder represented folders.

Best Regards,
Benjamin Domergue

On Tue, Jul 30, 2013 at 1:50 PM, Mike Abdullah [email protected]:

Wow, that's a pretty big batch of changes then. Which commit of iMedia
are you working from? It certainly doesn't appear to be the latest. Also,
what concrete "improvements" does this actually make?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/95#issuecomment-21785460
.

@mikeabdullah
Copy link
Collaborator

Right, I've slapped up a branch at https://github.com/karelia/iMedia/tree/benjamin-improve-rename-watching to get things started

@thebenjiman
Copy link

Super. If you ever need more information, let me know.
I have few interesting changes in Flickr too, but first I need my employer
approval.

Best Regards,
Benjamin Domergue

On Tue, Jul 30, 2013 at 7:22 PM, Mike Abdullah [email protected]:

Right, I've slapped up a branch at
https://github.com/karelia/iMedia/tree/benjamin-improve-rename-watchingto get things started

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/95#issuecomment-21807313
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gloubibou @thebenjiman @mikeabdullah