Skip to content

Add contributor guidelines for where to put source code in packages#49950

Merged
vvoland merged 1 commit intomoby:masterfrom
dmcgowan:contributing-package-location
Jun 10, 2025
Merged

Add contributor guidelines for where to put source code in packages#49950
vvoland merged 1 commit intomoby:masterfrom
dmcgowan:contributing-package-location

Conversation

@dmcgowan
Copy link
Member

@dmcgowan dmcgowan commented May 9, 2025

As discussed in maintainer call, before making large package and module moves, clearly layout the intended package structure in the contributing docs.

The layout is still open to discussion, the current idea is first we will create the api module, the client module, and then the daemon module. The root will continue to be me module-less for the next release to prevent new tags from breaking existing importers of package. More research is being still being done to measure the impact of making changes to the root module.

Related to #49873

@dmcgowan dmcgowan moved this from New to Needs maintainer feedback in Issue Triage May 15, 2025
CONTRIBUTING.md Outdated
- `docs` - All Moby technical documentation using markdown
- `hack` - All scripts used for testing, development, and CI
- `integration` - Testing the integration of the API, client, and daemon
- `integration-cli` - Testing the integration of the docker cli with the daemon
Copy link
Contributor

Choose a reason for hiding this comment

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

integration-cli tests are deprecated and we have a CI check that asserts that.
We should probably suggest adding an e2e in https://github.com/docker/cli instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats a good point, I didn't realize e2e was moved to the cli but makes sense

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

derp; had a pending review that I didn't submit; probably incomplete

CONTRIBUTING.md Outdated
- `hack` - All scripts used for testing, development, and CI
- `integration` - Testing the integration of the API, client, and daemon
- `integration-cli` - Testing the integration of the docker cli with the daemon
- `man`- All Moby reference manuals used for the `man` command
Copy link
Member

Choose a reason for hiding this comment

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

The man pages directly relate to the binaries produced for the daemon; we should make sure that these are part of the daemon module (as that must contain all the material used to produce the daemon packages)

- `client` - All Go files for the docker client
- `contrib` - Files, configurations, and packages related to external tools or libraries
- `daemon` - All Go files and packages for building the daemon
- `docs` - All Moby technical documentation using markdown
Copy link
Member

Choose a reason for hiding this comment

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

We'll likely need to split the docs per module as well; we have docs for the API, which should be part of the API module, and docs for the daemon and daemon configuration file (some of which still to be transferred back from the CLI repository). Given that those have a direct relation to each of those modules, should probably be split for each?

CONTRIBUTING.md Outdated
configuration and build files, no source files will be accepted in the root.

- `api` - All types shared by client and daemon along with swagger definitions.
- `bundles` - Autogenerated during build, do not check in files here
Copy link
Member

Choose a reason for hiding this comment

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

This one is in .gitignore, and not in the repository, so probably not worth including here?

CONTRIBUTING.md Outdated
- `docs` - All Moby technical documentation using markdown
- `hack` - All scripts used for testing, development, and CI
- `integration` - Testing the integration of the API, client, and daemon
- `integration-cli` - Testing the integration of the docker cli with the daemon
Copy link
Member

Choose a reason for hiding this comment

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

Probably should call out that no new tests should be added here. The remaining tests in this package should still be split between integration and/or moved to docker/cli e2e tests. Quite some tests are "abusing" the CLI to test the API (which should be in integration/) and some are testing CLI behavior (which should be moved to docker/cli);

Copy link
Contributor

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

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

LGTM, pretty straightforward coming from someone who didn't grow up with the historical organization. :)

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan force-pushed the contributing-package-location branch from 08055b0 to 071d27c Compare June 6, 2025 18:31
@vvoland vvoland merged commit 729cbbd into moby:master Jun 10, 2025
158 of 160 checks passed
@thaJeztah thaJeztah added this to the 28.3.0 milestone Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants