@discardableResult for becomeFirstResponder() and resignFirstResponder()

[bennokress]

When calling textField.becomeFirstResponder() or textField.resignFirstResponder() Xcode warns:

Result of call to 'becomeFirstResponder()' is unused
Result of call to 'resignFirstResponder()' is unused

In the example code this is avoided by prefixing all instances with _ = ..., but i like to keep my code clean, so the better solution is to add @discardableResult to those functions in source. This way the result can be used just like before, if needed, but it doesn't have to be used.

[intonarumori]

@bennokress Thanks for the contribution.

Your argument is valid, we should add @discardableResult to be consistent with the iOS SDK.

My personal preference would be to move @discardableResult to a separate line for readability. I looked around on Github and this seems to be the case with the majority of Swift developers as well.

Would you mind doing that change?

[bennokress]

done ;)

[intonarumori]

@bennokress Thanks 💥 👍

@k0nserv I think this one is safe to merge.

[intonarumori]

This PR brings us closer to iOS api consistency, great contribution

[k0nserv]

Grat contriubtion, thanks @bennokress. Could you please rebase on master and add a change log entry in CHANGELOG.md following the general pattern in that file too.

[bennokress]

i'm new to pull requests (this is my first approved one), so i'm asking before doing commands:

1. For the changelog: should I add a new version (v2.0.1) or add the description to v2?
2. If I'm making it v2.0.1 should I run git tag as 2.0.1?
3. Rebasing is also new to me: is there more to it other than executing git rebase master in Terminal?

[k0nserv]

No problem :)

1. Add it under the Master(Unrealsed) section. note I just added this so you will not see it in your fork until you've rebased
2. No I'll tag and release a version after the merge
3. So you need to make sure you are rebasing on this remote e.g Skyscanner/SkyFloatingLabelTextField not your own bennokress/SkyFloatingLabelTextField. You probably don't have this repo configured as a remote. You do this with git remote add upstream git@github.com:Skyscanner/SkyFloatingLabelTextField.git. When you've added this repo as a remote you can run git fetch --all do fetch all changes from all remotes. Then on master run git rebase upstream/master, this should apply cleanly(if anything goes wrongs or looks weird just do git rebase --abort to start over). Finally your branch will have diverged from what is on bennokress/SkyFloatingLabelTextField so you'll need to force push to overwrite the remote. You do this with git push origin master --force-with-lease

[bennokress]

ok, I hope I did everything correctly (had to add an SSH key first to fetch --all). I can't see my latest commit (CHANGELOG.md) in my own repo now, so let me know if everything worked ;)

[k0nserv]

Did you push your last commit? I can't see the CHANGELOG.md commit either. You can use the http git url for the upstream if you'd like. I prefer SSH urls personally.

[bennokress]

Yes, I did. git push was executed before i commented here, git push origin master --force-with-lease after your last answer. Maybe you see what I'm doing wrong here:

Sorry for causing trouble, I'm just really unexperienced with this.

[k0nserv]

Sorry for causing trouble, I'm just really unexperienced with this.

Not all, everyone starts out not knowing about things.

You just messed up the order of the add and the commit. You tried to commit before doing the git add. If you look closely it says "no changes added to commit".

Also don't use --force-with-lease unless you have just rebased or is required for some other reason. Force pushing is a destructive action, you are essentially telling the server that you know the result of the action will cause loss of data(which is fine when you are rebasing for example). You should be careful about force pushing in general, but it has some good use cases.

[bennokress]

I see, now it's working :)

[k0nserv]

Brilliant 👍 Thanks for the contribution

[k0nserv]

Just released version 2.0.1 with this change. Thanks again @bennokress for the contribution, great job! 👌

[bennokress]

happy to help 👍