Refactored code to remove unnecessary checks

[ghost]

When creating an immutable funnel, the code checks the value of "hasLengthByte" twice if it ends up being true. Checking for it once is all that is needed, as another branch can check for the other boolean statement in the if statement previously on line 1483. Differentiating the branches allows less redundant checks and does not introduce any side effects.

[ghost]

@swift-ci please test linux

[ghost]

@swift-ci please test

[ghost]

Fixed the mistake. Everything should be good!

[ghost]

@swift-ci please test linux

[ghost]

cc @gregomni

[xwu]

@pi1024e Could you please squash this into one commit?

[xwu]

@swift-ci Please test Linux

[ghost]

@swift-ci Please test linux

[millenomi]

@swift-ci Please test linux

[ghost]

Made those changes!

[drodriguez]

@pi1024e: instead of a lot of small changes, why don't we focus in the original change. I don't think anyone has any problem with the code as it is now, and we can figure out the other changes one by one.

Can you maybe rewrite this patch to only be those changes, squash, and then submit the other changes in different PRs?

Thanks.

[ghost]

Made those changes. Thank you!

[ghost]

Fixed!

[drodriguez]

@pi1024e: I don't know what your expertise level is, but I'm going to explain this as best as I can.

The only commit that we are currently interested is the first one, and also the fix from the first feedback. It seems that you are using the master branch for working, I would recommend the following procedure (the lines that start in # are comments by me, the lines that start with $ will have to be typed in the command line).

# Let's save all your work into a local branch
$ git branch cleanup-save
# Let's reset your master branch to the first commit
$ git reset --hard 8611fd80328d30e7fce86ba529152f4240a5aa41
# At this point edit the file CFString.c to move the line size += 1 to the bottom branch. Save your edit and don't modify anything else.
# I will be good to verify that only those changes are being sent.
$ git diff
# After checking that the changes are only the ones we want (the move of the line size += 1), we can continue.
$ git add CoreFoundation/String.subproj/CFString.c
# Let's rewrite the commit to have the fix.
$ git commit --amend --no-edit
# And finally update the remote to that version. I imagine your remote might be called origin, but replace <your remote name> in the line below by the name of your remote.
$ git push -f <your remote name>

That should get us only the initial changes, clean of every other commit. All your other changes are in a local branch called cleanup-save, and we can discuss about then in different PRs.

Thanks!

[ghost]

Thank you so much! Now I know what to do and how to properly contribute. Thank you!

[xwu]

Almost. You should try to follow @drodriguez’s instructions as exactly as possible; you’re still introducing new whitespace changes with every commit, possibly unintentionally.

[ghost]

Clang-format automatically made the whitespace changes. I thought I had to run that before committing to ensure no formatting changes would be done. Guess I was wrong.

[xwu]

It can be helpful to clang-format your own contributions, but you should not format existing code.

There are now pervasive changes in whitespace which cannot be taken as part of this PR. Again, please try to follow @drodriguez’s instructions as exactly as possible and make no other changes.

[ghost]

Everything is fixed!

[xwu]

This looks good. As the final step, please squash your commits into one commit.

Here is a tutorial that explains how:
https://thoughtbot.com/blog/git-interactive-rebase-squash-amend-rewriting-history

[ghost]

Squashed those commits!

[spevans]

@swift-ci test

[ghost]

Are these changes ok?

[xwu]

This looks good to me, but others still have to approve.

[xwu]

@swift-ci Please test

[ghost]

@swift-ci please test

[ghost]

Fixed!

[xwu]

Good, now if you squash the commit, this PR looks to be in better shape.

[ghost]

Fixed!

[ghost]

@swift-ci please test

[xwu]

I don't know what happened, but you've introduced additional changes again?

[ghost]

Done!

[ghost]

Okay. All changes done! Let us get this over with!

[ghost]

@swift-ci Please test

[xwu]

You need permissions to trigger testing. It's helpful if you've made sure that your changes compile and pass all tests locally.

@swift-ci Please test

[xwu]

@swift-ci Please test

[xwu]

@swift-ci Please test Linux