Skip to content
This repository was archived by the owner on Apr 3, 2018. It is now read-only.

Comments

docs: Developer: must develop/test in main repo#653

Closed
grahamwhaley wants to merge 1 commit intocontainers:masterfrom
grahamwhaley:20180302_devdoc_branches
Closed

docs: Developer: must develop/test in main repo#653
grahamwhaley wants to merge 1 commit intocontainers:masterfrom
grahamwhaley:20180302_devdoc_branches

Conversation

@grahamwhaley
Copy link
Contributor

You cannot build and test in a repo fork, as the sub-pkg
references do not move with the fork, and we fail due to
type difference failures.

Add details on how to add your user fork as a remote to the
main repo so you can develop/PR.

Also update details on installing required components into
/tmp, and the inherent weakness of that.

Note you need 'CI=true' to usefully run make check.

Fixes: #642

Signed-off-by: Graham whaley graham.whaley@intel.com


**Note:** As `virtcontainers` contains and references its own
[sub packages](https://github.com/containers/virtcontainers/tree/master/pkg),
it will **not** build or past its tests in your user fork repo (as the
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/past/pass/


In order to develop and submit PRs for `virtcontainers`, the easist method is to:

- create a user fork on github.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd capitalise the first words in all these bullets.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.


```
# make check
# CI=true make check
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you change all the prompts to "$ " as "# " reads as "OMG we're running as root dude!" to me ;)

[pkg references](https://github.com/containers/virtcontainers/blob/master/cni.go#L25)
will still reference the main repo, and this results in golang type clashes etc.).

In order to develop and submit PRs for `virtcontainers`, the easist method is to:
Copy link
Collaborator

Choose a reason for hiding this comment

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

s/easist/easiest/


In order to develop and submit PRs for `virtcontainers`, the easist method is to:

- create a user fork on github.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree.

You cannot build and test in a repo fork, as the sub-pkg
references do not move with the fork, and we fail due to
type difference failures.

Add details on how to add your user fork as a remote to the
main repo so you can develop/PR.

Also update details on installing required components into
/tmp, and the inherent weakness of that.

Note you need 'CI=true' to usefully run `make check`.

Fixes: containers#642

Signed-off-by: Graham whaley <graham.whaley@intel.com>
@grahamwhaley grahamwhaley force-pushed the 20180302_devdoc_branches branch from bf1ac20 to c3b142d Compare March 5, 2018 11:23
@grahamwhaley
Copy link
Contributor Author

@jodh-intel - updated, pushed.

@jodh-intel
Copy link
Collaborator

jodh-intel commented Mar 5, 2018

The only potential issue is the CI=true. So I'd say this is blocked on #654 until we've got agreement on how to use that variable. Aside from that issue,

lgtm

Approved with PullApprove Approved with PullApprove

In order to develop and submit PRs for `virtcontainers`, the easiest method is to:

- Create a user fork on github.
- Clone the main repo to your development machine (with `go get`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

missing -d ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, it was not an explicit $ got get -d -u <blah> instruction (similarly, did not call out how to clone repos or add remotes). So, nah, I think not.


# Cloning and Submitting

If you are just builiding `virtcontainers`, then you can simply clone the main repository
Copy link
Collaborator

Choose a reason for hiding this comment

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

/builiding/building/

from https://github.com/containers/virtcontainers.

If you wish to develop `virtcontainers`, you should fork the project on github under your
user.
Copy link
Collaborator

Choose a reason for hiding this comment

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

under your user account maybe?

@sboeuf
Copy link
Collaborator

sboeuf commented Mar 15, 2018

@grahamwhaley Need to be closed and moved to https://github.com/kata-containers/runtime/virtcontainers

@sboeuf sboeuf closed this Mar 15, 2018
@sboeuf sboeuf removed the review label Mar 15, 2018
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.

5 participants