gitpoint / git-point

GitHub in your pocket :iphone:
https://gitpoint.co/
MIT License
4.73k stars 788 forks source link

v1.4.0 Release - Final Checks #569

Closed housseindjirdeh closed 6 years ago

housseindjirdeh commented 7 years ago

Releasing 1.4.0 has been long overdue, and my apologies :O. Let's have this out before the end of this week ⭐️

Couple of things I've noticed so let's use this issue to track down all the changes we'll need before release 🚀

For any thing mentioned on the list here and on this issue you want to tackle, just tag this issue number on your PR.

On iOS && Android:

Profile images don't seem to be showing on profile screens

image

A few list items throughout the app have borders bolded a lot more

image

Notifications badge count has styling issues

View both screenshots above

Anything else anybody sees, please add it to this ticket. Shouldn't be too much regressions (and now that we're adding tests we should definitely be seeing less)

alejandronanez commented 7 years ago

@housseindjirdeh I can take a look at the profile picture bug.

housseindjirdeh commented 7 years ago

Thank you matey @alejandronanez <3

housseindjirdeh commented 7 years ago

⚠️ Thanks for logging this one @machour:

Android

image

App intro buttons not showing smoothly

FIXED 🎉

machour commented 7 years ago

@housseindjirdeh The app intro button should have been fixed by #527 Are you still getting this?

machour commented 7 years ago

@jouderianjr did you fix the cropped buttons for issues and pulls? I don't remember anymore :D

jouderianjr commented 7 years ago

yeah, #545 😄 Sorry for don't help much in this release, I'm pretty busy in this days 😢

housseindjirdeh commented 7 years ago

Thanks @machour, didn't run it and thought it was still showing for Android users. Thank you <3

housseindjirdeh commented 7 years ago

@yannbf was kind enough to log this in #536. Not sure if it's still occurring but logging it here to remind us to double check if it hasn't been resolved yet :)

image

alejandronanez commented 7 years ago

If anything related to 1.4 comes up we should add it to this board https://github.com/gitpoint/git-point/projects/2 I think it's easier to take a look there instead of an issue thread. What do you think guys?

housseindjirdeh commented 7 years ago

Create separate issues for each of these and put it on my board? Yep I think that's a good idea as well

chinesedfan commented 7 years ago

@housseindjirdeh I got a little confused about milestones. What's the relationship between projects and milestones?

andrewda commented 7 years ago

@chinesedfan Yea the line gets a little blurry. I'd say milestones are for tracking what issues we want in each release, whereas projects are for the current status of each issue (in progress, etc.) though I think we would be fine only using projects.

housseindjirdeh commented 7 years ago

^ Yep that's how I see it. Honestly perfectly fine with sticking to one as long as we all adhere as much as we can (and that can be projects).

Having milestones laid out as well shows the release number in the issues list screen which is pretty cool but isn't essential of course :)

machour commented 7 years ago

Just ran the app from master after a few days away, and I'm seeing a lot of obvious UI problems:

screen shot 2017-10-26 at 10 46 21 am screen shot 2017-10-26 at 10 47 42 am

I think the problem is that we've been accepting a lot of PRs for hacktoberfest, from people eager to contribute (and that's just great) but a bit sloppy with testing.

I believe its the responsibility of the person who decide to click the merge button to:

Could we all take some deep breaths before merging ? ❤️

housseindjirdeh commented 7 years ago

:(

Thank you @machour I appreciate it. I definitely think a lot of the folks submitting PRs to change our components styling aren't actually testing it on their simulator (maybe because it's simple to quickly grab an issue that requires a quick styling change and they have issues with getting it up and running on their simulator? I know it can be hard for Windows users for example)

I'm going to add a note to the contributing guidelines that asks for a screenshot of the changed component/screen before submitting. If any PR comes up that changes any JSX and/styles, please let's not merge it in unless there's a screenshot first.

After that - I agree. I would like the person who actually approves the PR to test it locally on their device/simulator and at least give a nod to other reviewers that it looks good. And I'm guilty here for merging PRs without having tested it with the assumption that other reviewers have, so my apologies 😞

If not a single person who approves it has tested it on their device/simulator - then I agree we should hold off merging.

alejandronanez commented 7 years ago

+1 on adding a screenshot before merging. I think the screenshot should be submitted by the person who creates the PR not by any of the people with merge permissions though, it can be really time consuming + people that send a PR should be responsible for their work.

@housseindjirdeh You can update the PR template with a checklist requiring the screenshot and some other goodies.

andrewda commented 7 years ago

@alejandronanez I agree that the person submitting the PR should be responsible for creating screenshots. There should also be exceptions, obviously, such as if a person can't get their environment setup correctly, and need help testing their changes.

machour commented 7 years ago

I fixed the user information list, but I encountered some difficulties with styled-components & react-native-elements. (More details in #586). Same things goes for the second screenshot of my previous comment, so I'm holding off on changes til we figure it out.

On a side note, I'm noticing contributors using styled-components names like ContainerBorderBottom. Same as CSS, I propose that we avoid names representing the layout as it will become incoherent as soon as we change the layout and use something more semantical like IssueDescriptionContainer. (this should be in the contributing guide as well IMHO).

alejandronanez commented 7 years ago

@gitpoint/maintainers do we have a list of all the regressions that happened after the SC refactor?

andrewda commented 7 years ago

@alejandronanez Good idea. That's probably the most thorough way to go about this.

machour commented 7 years ago

@alejandronanez I think we only mentioned what we've seen so far. The only way to get an exhaustive list would be to go through all merged PRs and test them one by one :/

jouderianjr commented 7 years ago

Hey everyone, I've noticed this change ( I have almost 100% sure that is related to react-native-elements update)

Before

screen shot 2017-10-28 at 20 27 02

After

screen shot 2017-10-28 at 20 16 01

Should we fix for the old style?( normal font weight and without ellipsize)

chinesedfan commented 7 years ago

@gitpoint/maintainers do we have a list of all the regressions that happened after the SC refactor?

I checked the merged 12 styled-component. Thank goodness that only 3 issues(#614, #615, #616) were added. For all SC related PRs, you can view by the styled-component label or #532.

SammyIsra commented 7 years ago

@jouderianjr Because Bios are usually longer than a line, I am for using the old style with normal font weight and without ellipsis.

andrewda commented 7 years ago

Agree with @SammyIsra - GitHub already limits the length of bios, so let's not limit it any further.

machour commented 7 years ago

@gitpoint/maintainers Just went through the various issues discussed in this thread and cleaned up the project board for 1.4.0: https://github.com/gitpoint/git-point/projects/2

I'm all in favor of starting to use the project board over milestones, as it gives a neat glimpse of the release status. 👌

housseindjirdeh commented 6 years ago

PR #665 open to cover a few styling updates

housseindjirdeh commented 6 years ago

There are a few things I'm noticing I would like to get to by tomorrow before releasing:

I'm noticing a weird flash when I navigate to the profile page:

flash

I also think issues seem to load/unload/load a bit much - should be able to simplify to show one loading indicator until screen is fully loaded:

stagger

Definitely not major and okay to hold these off to the next release if need be, but will be nice if we can cover them as well :)

machour commented 6 years ago

Yes, issue screen is getting really heavy (7 requests for a PR I think). I do however like to see comments right away when they are available (I'm on a poor connection and it sometimes can take up to 5 seconds to fully load).

This needs GraphQL sooo bad :)

jouderianjr commented 6 years ago

This needs GraphQL sooo bad :)

Totally agreed, GraphQL should be top priority for us for the next release 💃

housseindjirdeh commented 6 years ago

We really do :O

I am going to HAVE to dedicate sometime in the next few months implementing graphql where applicable. Fingers crossed 🙏

With that being said - just deployed v1.4 so going to close this.

machour commented 6 years ago

:tada: :tada:

rancidfrog commented 6 years ago

1 minor inconvenience: But, in preference it displays incorrectly:

machour commented 6 years ago

Thank you for noticing it @rancidfrog ! @housseindjirdeh could you update the composer.json file and release 1.4.1? (or else it will be nightmarish to understand the current version when the user reports a bug :D)

ocarreterom commented 6 years ago

package.json version is 1.3.0 in master :/

machour commented 6 years ago

yes, we need a step by step release guide to make sure we don't make this mistake again 😆

rancidfrog commented 6 years ago

Quick question, Is the apk in release section debug apk or release? Because, there is considerable lag which can be seen in video attached. Lag results in clicks being registered multiple times, for example near the end of video you see me clicking back 6 or more times from options only to have another options appearing repeatedly due to multiple clicks being registered when screen lagged. Might need a new issue, but just making sure. https://github.com/rancidfrog/project-info/raw/master/gp0.mp4

housseindjirdeh commented 6 years ago

@machour @rancidfrog apologies for missing that step during the release 😞 I just pushed out v.1.4.1 fixing that

@rancidfrog interesting - that should be the release apk 🤔. Thank you for logging this. I just put out 1.4.1 and this is how it renders on my simulator:

android

If it's not too much trouble, can you give v.1.4.1 a shot and let us know? Happy to open a separate issue if you're still experiencing it. Moreover, can anyone else please let me know if they're experiencing lag on their Android device with the latest version from the Play Store.

housseindjirdeh commented 6 years ago

Will close this for the meantime, let's address anything in newer tickets if we need to (but we can continue the discussion here :))