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.

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

v1.4.0 Release - Final Checks #569

Closed
housseindjirdeh opened this issue Oct 24, 2017 · 39 comments
Closed

v1.4.0 Release - Final Checks #569

housseindjirdeh opened this issue Oct 24, 2017 · 39 comments

Comments

@housseindjirdeh
Copy link
Member

housseindjirdeh commented Oct 24, 2017

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
Copy link
Member

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

@housseindjirdeh housseindjirdeh added this to To Do in v1.4.0 Oct 24, 2017
@housseindjirdeh housseindjirdeh changed the title v1.4.0 Release v1.4.0 Release - Final Checks Oct 24, 2017
@housseindjirdeh
Copy link
Member Author

Thank you matey @alejandronanez <3

@housseindjirdeh
Copy link
Member Author

housseindjirdeh commented Oct 24, 2017

⚠️ Thanks for logging this one @machour:

Android

image

App intro buttons not showing smoothly

FIXED 🎉

@machour
Copy link
Member

machour commented Oct 24, 2017

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

@machour
Copy link
Member

machour commented Oct 24, 2017

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

@jouderianjr
Copy link
Member

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

@housseindjirdeh
Copy link
Member Author

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

@housseindjirdeh
Copy link
Member Author

@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
Copy link
Member

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
Copy link
Member Author

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

@chinesedfan chinesedfan modified the milestone: Release 1.4.0 Oct 25, 2017
@chinesedfan
Copy link
Member

chinesedfan commented Oct 25, 2017

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

@andrewda
Copy link
Member

@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
Copy link
Member Author

^ 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
Copy link
Member

machour commented Oct 26, 2017

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:

  • checkout the proposed branch
  • carefully test everything that was impacted by this PR
  • only merge it if it's fully functional, or give a constructive feedback to the submitter (who will learn a lot from it)

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

@housseindjirdeh
Copy link
Member Author

housseindjirdeh commented Oct 26, 2017

:(

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
Copy link
Member

alejandronanez commented Oct 26, 2017

+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
Copy link
Member

@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
Copy link
Member

machour commented Oct 26, 2017

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
Copy link
Member

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

@andrewda
Copy link
Member

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

@machour
Copy link
Member

machour commented Oct 27, 2017

@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
Copy link
Member

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
Copy link
Member

@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
Copy link
Member

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

@andrewda
Copy link
Member

andrewda commented Nov 2, 2017

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

@machour
Copy link
Member

machour commented Nov 5, 2017

@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
Copy link
Member Author

PR #665 open to cover a few styling updates

@housseindjirdeh
Copy link
Member Author

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
Copy link
Member

machour commented Dec 12, 2017

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
Copy link
Member

jouderianjr commented Dec 12, 2017

This needs GraphQL sooo bad :)

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

@housseindjirdeh
Copy link
Member Author

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
Copy link
Member

machour commented Dec 15, 2017

🎉 🎉

@rancidfrog
Copy link

rancidfrog commented Dec 15, 2017

1 minor inconvenience:

But, in preference it displays incorrectly:

@machour
Copy link
Member

machour commented Dec 15, 2017

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)

@machour machour reopened this Dec 15, 2017
@ocarreterom
Copy link
Contributor

package.json version is 1.3.0 in master :/

@machour
Copy link
Member

machour commented Dec 15, 2017

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

@rancidfrog
Copy link

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 housseindjirdeh moved this from To Do to Completed in v1.4.0 Dec 19, 2017
@housseindjirdeh
Copy link
Member Author

@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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
v1.4.0
Completed
Development

No branches or pull requests

9 participants