search

interfasys / mediametadata

A cloud application which provides CRUD access to the metadata stored in images
GNU Affero General Public License v3.0
5 stars 1 forks source link

Minor changes to ImageDimension Class #20

Closed imjalpreet closed 8 years ago

imjalpreet commented 8 years ago

I have removed the JsonSerialize method for now.

Reviewer: @oparoz

oparoz commented 8 years ago

So, you've tested the code and everything still works without it? Info is logged and data stored in the DB? Maybe this was for your find command to retrieve the info?

imjalpreet commented 8 years ago

@oparoz Yes, I had tested the code and everything worked. But I didn't get your find command reference.

oparoz commented 8 years ago

I was thinking about this find command which retrieves the data: https://github.com/interfasys/mediametadata/blob/master/lib/Services/ImageDimensionMapper.php#L30

imjalpreet commented 8 years ago

@oparoz Yes, the find command is also not being used right now but I had written it to test whether the I am able to retrieve the data, which I have to in the future,

oparoz commented 8 years ago

OK, but then it shouldn't be a problem to keep these methods in.

imjalpreet commented 8 years ago

@oparoz Do you want to keep both the methods?

oparoz commented 8 years ago

If it's only testing code you won't use again, then you can remove them, but if they're going to be used later, then you can just leave them.

imjalpreet commented 8 years ago

I wasn't sure for json serialize, that's why I decided to remove it. But I think find may be useful in the future, that's why I think we can keep it.

oparoz commented 8 years ago

Then let's close this PR for now.