Skip to content

Conversation

@jatitoam
Copy link
Contributor

@jatitoam jatitoam commented Apr 6, 2017

No description provided.

@jatitoam jatitoam requested a review from puneet0191 April 6, 2017 16:02
Copy link
Member

@puneet0191 puneet0191 left a comment

Choose a reason for hiding this comment

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

Looks good to me, we could have provided the file path usage as a comment, to avoid the confusion of what we meant by _data folder, although it is clear. Thanks 😄

@jatitoam
Copy link
Contributor Author

jatitoam commented Apr 6, 2017

Fixed (and added cs too)

@puneet0191
Copy link
Member

Thanks 👍

@puneet0191
Copy link
Member

@yvesh I will be merging this one, Please review..I have tested the code on my local machine.

@yvesh
Copy link
Contributor

yvesh commented Jun 21, 2017

@puneet0191 sure, fine for me. I am not sure why we have to output the extension type (some are also missing), but rest looks fine :-)

@jatitoam
Copy link
Contributor Author

About the extension, it's because Joomla outputs different messages depending on the extension types. I can't recall if there are other messages out of those 3 so I think that's it.

I'm going to merge this since I'm ok with it too.

@jatitoam
Copy link
Contributor Author

Nevermind, there are new cs errors :) can't merge even with the approval, so I'll fix

@jatitoam
Copy link
Contributor Author

Replaced by #138

@jatitoam jatitoam closed this Aug 23, 2017
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