Moving BUILD file to the proper place. - Githubissues

AliSoftware / OHHTTPStubs

Stub your network requests easily! Test your apps with fake network data and custom response time, response code and headers!

MIT License

5.03k stars 602 forks source link

Moving BUILD file to the proper place. #330

Closed jeffctown closed 3 years ago

jeffctown commented 3 years ago

Description

Moving the BUILD file to it's proper place.

jeffctown commented 3 years ago

@thii - Just want to confirm this is the right spot.

jeffctown commented 3 years ago

Looks like I can't merge my own PR unless someone other than me with write access approves it. @AliSoftware lil help?

AliSoftware commented 3 years ago

I'm happy to approve; but I have no idea what this PR is about and what this BUILD file is… 😅

jeffctown commented 3 years ago

It's adds support for Bazel, which is Google's alternative build system. I'm not very familiar with it either, but this is what it is if you're interested.

AliSoftware commented 3 years ago

Ahhh ok. Do we have a CHANGELOG entry for this and a mention in the README or CONTRIBUTING that we're supporting Bazel?

thii commented 3 years ago

In the original PR (long ago), originally I just intended to add a missing import (so that we wouldn't have to patch it), but ended up adding a BUILD file upon a suggestion. It was somewhat incomplete now that I'm looking at it again. People who use Bazel to build their iOS app often have custom rules and write BUILD files themselves, so I'm not sure if you still want to keep and provide support for it.

AliSoftware commented 3 years ago

Never having found the time myself to dig into Bazel (even though I've always wanted to try) I personally won't provide support myself, but I'm not the only maintainer of this OSS repo anymore so if anyone else is comfortable enough and able to provide support I don't see any reason not to keep it.

I'm just a bit sad that be Bazel didn't choose a more descriptive name for that file like Bazelfile (to be more aligned with what every other tooling is naming their config files), since anyone not familiar with Bazel won't know what this file is about. I think if possible we should add a comment at the top of this BUILD file to document what it is at least?

thii commented 3 years ago

Bazel also supports BUILD files named BUILD.bazel, so we can use that.

AliSoftware commented 3 years ago

Perfect, let's update the name of the file then, and then we can merge this!

thii commented 3 years ago

Sorry for the back and forth. Upon a second thought, I think we should remove the BUILD file from the repo. The reasoning being:

It is incomplete—it only includes the Objective-C sources, so it might not work for all users.
There's currently no canonical way of building a mixed Objective-C & Swift target in Bazel (it's possible, but everyone does their own way).
The existence of it prevents the users from using a custom rule for it by adding a BUILD file to an uppper directory (because it makes the enclosing directory a package, and glob does not match files in subpackages).

AliSoftware commented 3 years ago

Fair enough 👍

jeffctown commented 3 years ago

Of course I'll wait for tests to pass before merging, but this is otherwise good to go @AliSoftware .

AliSoftware commented 3 years ago

Thanks for taking care of this. Feel free to merge in green.