kiwi-bdd / Kiwi

Simple BDD for iOS
BSD 3-Clause "New" or "Revised" License
4.14k stars 512 forks source link

Rename any() macros to kw_any() #701

Closed SergeyKuryanov closed 6 years ago

SergeyKuryanov commented 6 years ago

This address issue: #698

As any other solution was provided, simple macros renaming.

orta commented 6 years ago

This is a major semver bump /cc @sharplet

SergeyKuryanov commented 6 years ago

Tests pass locally, not sure why beforeAllSpecs, afterAllSpecs should be executed.

ecaselles commented 6 years ago

Agree @orta.

@SergeyKuryanov Thanks for doing this! I was thinking... would it be possible to check if any is defined? In case it is, definekw_any instead. Otherwise, keep defining any to keep backwards compatibility.

SergeyKuryanov commented 6 years ago

@ecaselles
Checking for any defined is not working for me, seems like any macros setting appear before Project-Swift.h definitions, so that check won't help.

Something like #if __has_attribute(external_source_symbol) is working, but I don't think this is reliable check.

ecaselles commented 6 years ago

Thanks @SergeyKuryanov, so... I guess we will have to introduce this as a breaking change, won't we? @orta @sharplet how do you feel about it?

orta commented 6 years ago

I don't hold much sentiment on major version bumps ( Danger is on 5 for example )- this sounds like something people need to keep up to date with Xcode's ever changing build tooling , so we shouldn't be precious.

Do it IMO

SergeyKuryanov commented 6 years ago

@ecaselles I don't like Idea of breaking change, but this only one way to make it work (at least I didn't found another). So I guess we should do it.

ecaselles commented 6 years ago

Fair enough! Thanks @orta and @SergeyKuryanov. We’ve got a decision 👌

ecaselles commented 6 years ago

Tests pass locally, not sure why beforeAllSpecs, afterAllSpecs should be executed.

@SergeyKuryanov after investigating a bit, I believe the tests fail because of the check done in test_suite_configuration.rb, which expects Xcode's logs to contain mentions to beforeAllSpec and afterAllSpec. I believe its purpose is just to check these two lines run. Not sure why it is not being printed any more though or how would this PR break it. Maybe @sharplet could help us here, since he was the one introducing the check.

In the meantime, I am downloading Xcode 7.3 to see if I can reproduce it locally, but I am not sure why that would be a problem in this PR. I would like to be able to reproduce / understand how it was originally working 🤔

ecaselles commented 6 years ago

After having a chat with @sharplet, I think I have a better understanding of why this might be failing. I still need to check a couple things before I can resolve it. I will keep this thread updated, but I will continue working on #703, in order to keep this one clean.

ecaselles commented 6 years ago

Solved the issue with tests (#703). Now this requires a rebase on top of master.

ecaselles commented 6 years ago

Closed in favour of #704 (same changes, rebased on top of master so the build passes).