search

smarty / assertions

Fluent assertion-style functions used by goconvey and gunit. Can also be used in any test or application.
Other
100 stars 34 forks source link

Added power support for the travis.yml file with ppc64le. #41

Closed sreekanth370 closed 3 years ago

sreekanth370 commented 4 years ago

Added power support for the travis.yml file with ppc64le. This is part of the Ubuntu distribution for ppc64le. This helps us simplify testing later when distributions are re-building and re-releasing

mdwhatcott commented 3 years ago

@sreekanth370 - Can you provide more context for why this is necessary? I'm not really concerned about merging it, but I am concerned that if we ever rewrote the travis yml file we might not remember why these directives are needed and they would be at risk for removal.

sreekanth370 commented 3 years ago

For more info tag @gerrith3

gerrith3 commented 3 years ago

Hey @mdwhatcott , not sure how familiar you are with the Power architecture - primarily the bigger enterprise class systems from IBM for banking/insurance/retail/etc.etc but this ensures that your code and all dependencies are tested on the Power architecture. Debian/Ubuntu, and Fedora/RHEL/CentOS have ppc64le (Power) distributions and this code goes into those distributions. This ensures that top of tree is always validated on ppc64le using (free) Power servers at Travis CI.

As far as forgetting, @sreekanth370 is it possible to add a few comments to clarify what you changed?

sreekanth370 commented 3 years ago

added the power support on travis.yml and test was succeeded for amd64, ppc64le. kindly merge this transaction for master travis.yml

arch:

kindly find the test results: https://travis-ci.com/github/sreekanth370/assertions/builds/196266791

mdwhatcott commented 3 years ago

@sreekanth370 - the suggestion from @gerrith3 to document this addition is, I think, a good one. We, at SmartyStreets, have no need to support ppc64le architecture at this time and our memory of the conversation of this PR will fade with time...

joliver commented 3 years ago

It seems like the best solution here is a fork of the library. We don't have any platform-specific code in this library so the addition of platform-restrictive testing (amd64, and other architectures) seems like a step in reverse.

gerrith3 commented 3 years ago

@joliver I don't get the 'step in reverse' - this package is included in multiple distros that are already built for ppc64le. At one point the distros either ported this code via a fork (usually they return the changes) or the code just worked. Ad @mdwhatcott said, at least documenting that it works, and verifying it is free, it allows a base of users to make use of it on Power, and documents that it at least works. You may also want to add a little note in the README that says "this is expected to be platform independent, tests run on {architecture list} and the community is free to use the code without support (or maybe just "with community support"). I prefer the latter - for instance when we have customers/ISVs/end users of any sort that need this, if it doesn't work, we will typically help do the port if possible. We aren't always the experts, though, and may reach out for community help when doing so. That doesn't mean that smartystreets is supporting in for ppc64le but it is (like most open source projects) use at your own risk, Your Mileage May Vary.

mdwhatcott commented 3 years ago

@sreekanth370 @gerrith3 - Ok, here are several thoughts for consideration.

We didn't foresee this project being brought into Linux distributions and frankly, it's not a priority for us to support those choices. We have plans to migrate away from travis CI to github actions. I have no idea what effect that migration will have on you or whoever else has come to depend on this library.

I guess I'm still not really sure why this tiny addition to the travis config is necessary for you. This code is just a library and it doesn't have any architecture-specific code. Obviously you have code that depends on this library--doesn't it only really matter that you can compile that project on whatever architecture needed?

Can you be more specific about what problem you are trying to solve and how this change helps?

gerrith3 commented 3 years ago

@mdwhatcott :

sudo apt-cache search smartystreets golang-github-smartystreets-assertions-dev - fluent assertion-style functions golang-github-smartystreets-go-aws-auth-dev - Go library to signs requests to Amazon Web Services golang-github-smartystreets-goconvey-dev - Go testing in browser - sources golang-github-smartystreets-gunit-dev - xUnit-style test fixture adapter for go test

gerrith3 commented 3 years ago

Oh, and that's on a ppc64le machine.

gerrith3 commented 3 years ago

Re: github actions - we are planning to add Power hardware there for testing as well, there is a little matter of connecting GHA to a cloud backend right now. We don't yet have a solution available for that.

mdwhatcott commented 3 years ago

So, what problem are we trying to solve?

gerrith3 commented 3 years ago

@mdwhatcott reducing the number of servers we send to distros for testing, moving testing to the cloud, auto-refresh hardware (through updates in the cloud), catching defects as they happen (as opposed to months later), testing infrastructure, ensuring that ppc64le users have a first class experience just like amd64 users, to name a few...

sreekanth370 commented 3 years ago

kindly respond and merge the code with power architecture. For more info tag @gerrith3