ez-net-app icon indicating copy to clipboard operation
ez-net-app copied to clipboard
verified publisher icon CodeLanka

Fix important issues and start updating code base

Open TheLukaszNs opened this issue 8 years ago • 16 comments

Fast summary

This PR resolves #45, resolves #47, closes #48 and finally resolves #51.

Code base update:

  • [x] <ResponsiveImage /> -> <Image />
  • [x] Moved images from img to assets/images
  • [x] ~~Updated React.PropTypes with prop-types (this was actually done - we didn't use old PropTypes from React)~~ Deleted all instances React.PropTypes
  • [x] Moved all styles to assets ~~(I don't know if we should move all styles from its' components to separated files and create index file to store them all?)~~
  • [x] Made additional styles update which is not actually code base update 🤔

TheLukaszNs avatar Feb 19 '18 18:02 TheLukaszNs

I was already on it dude. That was my issue. I'd suggest that it would have been better if u would have commented on that issue if you were on it, I would not have worked on it. 😄

vibhavagarwal5 avatar Feb 19 '18 18:02 vibhavagarwal5

Oh, I saw You were working on caching images, weren't you? I didn't touch this issue.

TheLukaszNs avatar Feb 19 '18 19:02 TheLukaszNs

I see You done much more work than me. I'll close my PR 😃

TheLukaszNs avatar Feb 19 '18 19:02 TheLukaszNs

Yeah i am working on it. But still that if u would have commented on it that u were taking it, i would not have spent time on it. Since that issue was opened was me, it was my duty to close it asap. 😄 Thats why i worked on it. I hope u dont mind. 😄

vibhavagarwal5 avatar Feb 19 '18 19:02 vibhavagarwal5

Thanks @TheLukaszNs

vibhavagarwal5 avatar Feb 19 '18 19:02 vibhavagarwal5

@TheLukaszNs @vibhavagarwal5 Hope you two can work jointly on this. Use https://gitter.im/CodeLanka/ez-net-app for fast discussions, I too will be there.

agentmilindu avatar Feb 20 '18 05:02 agentmilindu

@agentmilindu I think this is resolved.

vibhavagarwal5 avatar Feb 20 '18 05:02 vibhavagarwal5

I am reopening this PR as of additional issues have been opened - I had to fix them and I had my code base already updated by me. I'll push patches to develop in the next minutes.

TheLukaszNs avatar Feb 21 '18 20:02 TheLukaszNs

Aren't these exactly the changes i have done? Looks duplicate to me.

vibhavagarwal5 avatar Feb 21 '18 20:02 vibhavagarwal5

Yeah it's very similar as of it couldn't be done more differently. But if you make a closer look you will notice that it's made in the other way.

It also fixes #51

TheLukaszNs avatar Feb 21 '18 21:02 TheLukaszNs

Ummm, I'm bit confused here. I see three PRs which kinda looks similar. Can you two please guide me through?

agentmilindu avatar Feb 22 '18 14:02 agentmilindu

@agentmilindu PR #52 is very much different from the other two. Its regarding the caching issue. Whereas PR #49 and #50 is about the code updation ie. issue #47 . Since I submitted it first with major changes, and then shortly after @TheLukaszNs submitted a similar PR, its looks duplicate to mine.

vibhavagarwal5 avatar Feb 22 '18 14:02 vibhavagarwal5

@vibhavagarwal5 I've submitted this PR first (10 mins before you). Then I decided to close it, because You did nearly the same work as me, but - you are right here - you've created issue #47 (however you did not inform that you'll do it). Yesterday I reopened this issue, merged changes in mine fork and updated this PR.

@agentmilindu those PRs are nearly the same because we were working on the same problem, but I've also fixed #51 and did some styles update.

TheLukaszNs avatar Feb 22 '18 14:02 TheLukaszNs

Is issue #51 really an issue?

vibhavagarwal5 avatar Feb 22 '18 15:02 vibhavagarwal5

Let me go through the PRs and see.

agentmilindu avatar Feb 22 '18 17:02 agentmilindu

@nushydude hey, I need your help here to get these two PRs merged.

agentmilindu avatar Oct 04 '18 04:10 agentmilindu