1419650 - Use type inference where possible to remove unneeded static type specifications

Status: RESOLVED FIXED

(Firefox for iOS :: General, enhancement)

Description

[adamnemecek]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36

Comment 1

[James Hugman [:jhugman] [@jhugman]]

Attachment: Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/3558

Comment 2

[Farhan Patel [:farhan]]

Comment on attachment 8939543 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/3558

I'm okay with this patch. I was hesitant about removing the class names so if you think we should have them then lets merge this patch with the class names added back in. 

As for the rest of the changes being individual commits we squash and rebase anyways.

Comment 3

[James Hugman [:jhugman] [@jhugman]]

> I was hesitant about removing the class names so if you think we should have them then lets merge this patch with the class names added back in. 
Reading through the patch, I was persuaded from leaving them in to removing them. 

There are still a bunch of class names still in selectors. These may be necessary, granted.

I'm also of a mind we should land this ASAP before it gets too bit rotted.

> As for the rest of the changes being individual commits we squash and rebase anyways.
This is fair comment.

Comment 4

[James Hugman [:jhugman] [@jhugman]]

Comment on attachment 8939543 [details] [review]
Link to Github pull-request: https://github.com/mozilla-mobile/firefox-ios/pull/3558

You want to land this :fpatel?

Comment 5

[adamnemecek]

I'll fix up the class names in the next pull request. But, yes, not all of them can be removed but most can. I'll also fix the notification.names.

How quickly can you guys on average merge my PRs in?

Comment 6

[Farhan Patel [:farhan]]

Sorry for the delay in merging PRs. We've all been away for the holidays and some of us had taken extended time off after as well. 

Im hesitant to merge a PR in if the next PR is just going to revert changes. It makes it harder to follow history when looking back through git.

If you could fix them before we merge this PR I think it'll help churn and make the codebase better. Im sorry you had to spend time working on this. We should totally have a better defined style guide to help someone like yourself. 

I'm cool with leaving the NSURLExtension changes and other small nits that were fixed. 


As soon as the build is green I can merge it in.

Comment 7

[Farhan Patel [:farhan]]

Sorry I think I misunderstood James' feedback. I thought he was asking to revert some changes. But it looks like he just wanted some more simplifying. In that case you're right lets leave it for the next PR. 


There are some merge conflicts that need to be fixed once those are fixed we can merge the patch in. If you like you can give me access to the branch and I can fix the conflicts and land :)

Comment 8

[adamnemecek]

Ok, I invited you.

Yeah, I figured most people were out.
But we should still talk about how to contribute. Like the overhead of opening a new PR is quite a bit. Is there maybe a slack channel that I could join to coordinate better?

Comment 9

[Farhan Patel [:farhan]]

merged! I think irc on #mobile https://wiki.mozilla.org/IRC or in GH comments is the best way.

Comment 10

[James Hugman [:jhugman] [@jhugman]]

Thanks for your contribution! Every little helps, and this PR was pretty chunky. Many thanks.

Comment 11

[Farhan Patel [:farhan]]

Attachment: Pull Request