docs: add a developing.md #1752
Conversation
| ## Running tests | ||
|
|
||
| ``` | ||
| go test ./... |
There was a problem hiding this comment.
What about scripts to run all tests? Or will we update this doc later once those are merged?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah I misunderstood your question. If we add scripts for multiple modules, I can update this then. This doc is capturing what we need/have today.
| - [Go 1.12 or Go 1.13](https://golang.org/dl) | ||
|
|
||
| To run some tests these tools are also required: | ||
| - PostgreSQL 9.6+ server running locally, or set [environment variables](https://www.postgresql.org/docs/9.6/libpq-envars.html) (e.g. `PGHOST`, etc) for alternative host. |
There was a problem hiding this comment.
There's also the development workflow using the new docker-compose stuff. Is that out of scope for this PR?
There was a problem hiding this comment.
Yup, that's out-of-scope. In this PR I really wanted to just get the instructions for managing dependencies out there, and it felt odd to have them be the only instructions in this file, so I added some basic filler.
@tamirms maybe you could link to docker-compose instructions from in here once that's ready for promoting, if it isn't already?
|
@leighmcculloch shall we point in this guide to each sub-repo developing guidelines? |
|
@abuiles Good idea! We should probably normalize these all a little, but I can add some links for the moment. |
Add reference in the `DEVELOPING.md` to the Horizon docs that including more specific developing instructions. To guide contributors to the right place. #1752 added general developing documentation and @abuiles pointed out in #1752 (comment) post-merge that there is detailed developing documentation for Horizon that we should guide folks to.
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
needed with deprecations, added features, breaking changes, and DB schema changes.
semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
Summary
Add a
DEVELOPING.mdcapturing some basic instructions on how to develop in this repository, including instructions on how to use Modules to add, update and remove dependencies.Goal and scope
To make a clear path for folks to add, update and remove dependencies. While we mostly use the standard Go modules features to manage dependencies they are relatively new in the ecosystem and it is helpful if we anyone adding dependencies knows how to get it right first time without needing to iterate, receive CI or PR feedback, and then do more work.
We also use a non-standard go.list file so that we can see what our actual build/tested dependency versions are in PR diffs, according to
go list -m all.CI will error if any steps haven't been followed for updating dependencies, but as @graydon pointed out when he was our first person doing an upgrade, it's helpful if we can give developers clear instructions so they have the opportunity to get it right and so it's not an unknown they're stepping into.
It's worth addressing that there is some overlap of these instructional files, e.g.
README.md,DEVELOPING.mdandCONTRIBUTING.md, but each has a different target audience and are relevant at different stages.README.md: Folks learning about our code, importing the Go SDK, installing from source.DEVELOPING.md: Developers wanting to make changes to the code.CONTRIBUTING.md: Developers wanting to contribute changes back to this repo.For this reason it would be overwhelming to dump all this information into
README.md, or to clutterCONTRIBUTING.mdwith details that are no longer relevant to a developer when they are thinking about opening a PR.This is a follow up to #1634.
Summary of changes
DEVELOPING.mdREADME.mdKnown limitations & issues
N/A
What shouldn't be reviewed
N/A
Merging
This PR is intended to be merged to
masterafter #1751 which already contains commit cb3300f.