-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add .papr support #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
CI is running, this is very promising |
63bb055 to
c91c78c
Compare
|
baaa...lessee whahappeeen... |
yourfault.../me runs away 😀 (srsly, giving a once-over of changes here...) |
|
We're doing install.tools which should grab the linter. Maybe it's in the wrong place? |
|
No idea why this closed. |
|
I don't have permission to reopen - you're repo owner |
cevich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some things to thinkabout at least...
.papr.sh
Outdated
|
|
||
| export GOPATH=$HOME/gopath | ||
| export PATH=$HOME/gopath/bin:$PATH | ||
| export GOSRC=$HOME/gopath/src/github.com/projectatomic/libpod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, is $HOME appropriate here? I dunno what "user" is running the job or where it starts from. Maybe I'm wrong though, it's worth double checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was copied from buildah, I think in CI this will be /root
.papr.sh
Outdated
|
|
||
| (mkdir -p $GOSRC && cd /code && cp -r . $GOSRC) | ||
|
|
||
| dnf install -y \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I this ever going to run against a RHEL system? Hmmm....n/m it's probably not a problem that needs solving now.
.papr.sh
Outdated
|
|
||
| # PAPR adds a merge commit, for testing, which fails the | ||
| # short-commit-subject validation test, so tell git-validate.sh to only check | ||
| # up to, but not including, the merge commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from buildah
.papr.sh
Outdated
| # short-commit-subject validation test, so tell git-validate.sh to only check | ||
| # up to, but not including, the merge commit. | ||
| export GITVALIDATE_TIP=$(cd $GOSRC; git log -2 --pretty='%H' | tail -n 1) | ||
| export TAGS="seccomp $(hack/btrfs_tag.sh) $(hack/libdm_tag.sh) $(hack/btrfs_installed_tag.sh) $(hack/ostree_tag.sh) $(hack/selinux_tag.sh)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an old-fuddy-duddy, I like having all my "constants" all at the beginning, so I always know where to find them. I dunno though for this context, maybe that doesn't make sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem moving but again this came from buildah. We should probably be consistent.
.papr.yml
Outdated
| branches: | ||
| - master | ||
| - auto | ||
| - try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, not sure why you need anything other than 'master'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Me neither. I will remove auto and try.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are for homu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Mostly we just use the auto branch - when doing a merge, it pushes there first and waits for PAPR to report)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's a "homu"? Can you eat it? 😀
@rhatdan I checked the docs, this is just the branches to test, so just 'master' is appropriate here since none of the other branches exist.
/me still wonders what a homu is
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cevich Homu will create those branches when doing a merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooohhh, https://github.com/servo/homu/ it's a service. gotcha, kk, I'll put those back and add a comment 😀
Makefile
Outdated
| @@ -146,7 +146,7 @@ install.man: | |||
| install ${SELINUXOPT} -m 644 $(filter %.8,$(MANPAGES)) -t $(MANDIR)/man8 | |||
|
|
|||
| install.config: | |||
| install ${SELINUXOPT} -D -m 644 seccomp.json $(ETCDIR_CRIO)/seccomp.json | |||
| install ${SELINUXOPT} -D -m 644 seccomp.json $(ETCDIR_LIBPOD)/seccomp.json | |||
| install ${SELINUXOPT} -D -m 644 crio-umount.conf $(OCIUMOUNTINSTALLDIR)/crio-umount.conf | |||
|
|
|||
| install.completions: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did not review...all JFM to me 😀
plugins: add plugins from containernetworking/cni
Signed-off-by: Daniel J Walsh dwalsh@redhat.com