Skip to content
This repository has been archived by the owner on May 6, 2020. It is now read-only.

Make opensuse build #37

Merged

Conversation

thiagomacieira
Copy link

@thiagomacieira thiagomacieira commented Aug 5, 2017

@thiagomacieira
Copy link
Author

Each of the commits updates multiple subsystems, like a6e9e1e. Any way to convince Travis to accept it anyway?

@gorozco1
Copy link

gorozco1 commented Aug 5, 2017

@thiagomacieira Thanks a lot for this PR. The missing part on the commit and the PR is the word Fixes:

You need to add

Fixes #37 to make travis happy

#37 is the issue I filed yesterday related to this.

@gorozco1
Copy link

gorozco1 commented Aug 5, 2017

@thiagomacieira oh, and I noticed that you are trying to push 3 commits.

Could you squash them in a single commit and add the issue the commit fixes (In this case #37 )

It is recommended that each of your patches fixes one thing, so in this case your 3 commits fixes one single issue. So better be in a single squashed commit.

@thiagomacieira
Copy link
Author

Well, each of the patches fixes one problem with the .spec files. Building for OpenSUSE wasn't a simple fix.

Do you really want me to squash?

@gorozco1
Copy link

gorozco1 commented Aug 5, 2017

@thiagomacieira oh, if each of the commits fixes an issue could you please create issuesand then reference the issue fixed in each of the commits as: Fixes: #XX

@thiagomacieira
Copy link
Author

#36 is a "meta" task. I am saying that there were three different problems in providing OpenSUSE packages.

Do you want me to file three issues for the small details that I discovered, fix each with one commit, then close #36?

@gorozco1
Copy link

gorozco1 commented Aug 6, 2017

@thiagomacieira,

On a closer look I found that travis is complaining that your commits does not have a subsistem in the subject.

Failed to find subsystem in subject: "Update .spec files to add BuildRequires for OpenSUSE"

From https://github.com/clearcontainers/tests/tree/master/cmd/checkcommits

The commit subject contains a "subsystem" (adjust using --subject-length=). A "subsystem" is simply one or more words followed by a colon at the start of the subject and designed to identify the area of the code the change applies to.

Could you try to rephrase the subject in your commits with something like:

  • opensuse: Add directory names to the %file listing
  • opensuse: Update libexec dirs to match OpenSUSE installation guidelines
  • opensuse: Update .spec files to add BuildRequires for OpenSUSE

Some are just things that the default image for OpenSUSE doesn't have,
while the Fedora/CentOS/RHEL ones do. Some others are different package
names compared to the Red Hat distributions.

The %if trick was taken from
https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
OpenSUSE has no /usr/libexec. Those go simply to /usr/lib.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
OpenSUSE requires the directories to be owned too, not just the files. I
don't know if this is against Red Hat packaging guidelines, so I didn't
use any %if.

Fixes clearcontainers#36.

Signed-off-by: Thiago Macieira <thiago.macieira@intel.com>
Copy link

@gorozco1 gorozco1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@gorozco1 gorozco1 requested a review from amshinde August 6, 2017 20:44
@jodh-intel
Copy link

lgtm

@jodh-intel jodh-intel merged commit 59c3de5 into clearcontainers:master Aug 7, 2017
@erick0z erick0z mentioned this pull request Aug 8, 2017
egernst pushed a commit to egernst/cc-packaging that referenced this pull request Aug 14, 2018
Build qemu without librados. This allows build qemu in OBS.

Fixes: clearcontainers#37

Signed-off-by: Jose Carlos Venegas Munoz <jose.carlos.venegas.munoz@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants