Document first-time developer setup, add conda option #2083

zmbc · 2024-03-19T22:09:42Z

Type of PR

BUG
FEAT
MAINT
DOC

Is your Pull Request linked to an existing Issue or Pull Request?

#2019

Give a brief description for the solution you have provided

More or less followed the outline I gave in #2019 (reply in thread), except I reordered some things.

PR Checklist

Added documentation for changes
Added feature to example notebooks or tutorial (if appropriate)
Added tests (if appropriate)
Updated CHANGELOG.md (if appropriate)
Made changes based off the latest version of Splink
Run the linter

CONTRIBUTING.md

RobinL

Thanks very much this is great. A few minor comments.

I going to ask Tom to look at this too, just to get a second pair of eyes on a couple parts of this, but overall, very happy with how it's looking. A big improvement!

docs/dev_guides/changing_splink/testing.md

RobinL · 2024-03-20T07:59:35Z

docs/dev_guides/index.md


 When making changes to Splink, there are a number of common operations that developers need to perform. The guides below lay out some of these common operations, and provides scripts to automate these processes. These include:

-* [Building a Virtual Environment](./changing_splink/building_env_locally.md) - to replicate the conditions when Splink is installed by users.
 * [Linting and Formatting](./changing_splink/lint_and_format.md) - to ensure consistent code style and to reformat code, where possible.


Should the first bullet in this list point to the new quickstart page?

I don't see a ton of utility in this list. "Linting and Formatting" and "Testing" are already linked in the contributor's guide section about contributing code (the only type of contribution to which they are relevant). Likewise with building the docs locally.

Releasing a new package version is nice to have documented, but calling it out for new contributors seems unnecessary -- they almost certainly wouldn't have the permissions to do this.

And contributing to the blog seems like perhaps it should just be its own section in the contributor's guide.

Yeah agreed, in fact the whole page feels unnecessarily repetitious and could probably be dropped entirely.

We probably need to review it separately. Just want to make sure the quickstart is as obvious as possible - feel free to do whatever you think's best!

Sounds good and happy to help with that. I think in this PR I won't put the quickstart in the list since it is not a "common operation that developers need to perform."

docs/dev_guides/changing_splink/development_quickstart.md

scripts/conda/development_setup_with_conda.sh

docs/dev_guides/changing_splink/development_quickstart.md

RobinL · 2024-03-20T09:22:41Z

@ThomasHepworth would you mind having a quick look at this? I've gone through in detail, but I think it'd be good to get a second pair of eyes specifically on docs/dev_guides/changing_splink/development_quickstart.md

I have:

Built the docs locally and they look fine to me
Worked through the conda instructions, got the conda environment working and tests running within it

So I think it's just a sense check that's needed

ThomasHepworth · 2024-03-20T12:07:18Z

Thank you for doing all of this!

It's really useful for us to get input and feedback from people coming to Splink from outside our bubble. I'll check it over now and leave a few comments.

ThomasHepworth

Thanks again Zeb! This is a great step forward in this department.

See a few minor comments and discussion points below. I've also had a word with Robin espousing the use of conda and I believe he's going to comment on that side of things.

docs/dev_guides/changing_splink/managing_dependencies.md

scripts/create_venv.sh

scripts/conda/development_environment.yaml

docs/dev_guides/changing_splink/testing.md

RobinL · 2024-03-20T16:34:19Z

Tom and I had a chat about this earlier. We both agree it's a great improvement.

There's one aspect both of us picked up on: Whilst the quickstart page contains options for both conda and non-conda, the way its layed out implicitly suggests that conda is the recommended option. This didn't feel quite right because none of the core dev team use conda.

You'll see I've done a draft PR to move things around a little and include a (hopefully not too controversial) line on whether re recommend conda or not.

Co-authored-by: Robin Linacre <robin.linacre@digital.justice.gov.uk>

Co-authored by: Tom Hepworth <45356472+ThomasHepworth@users.noreply.github.com>

Co-authored by: Robin Linacre <robin.linacre@digital.justice.gov.uk>

…hub.com/zmbc/splink into docs/onboarding-development-environment

zmbc · 2024-03-21T16:21:49Z

Thank you both for your reviews! I believe I've addressed all the comments now. See my comments on the de-emphasize conda PR -- if they look good to you, I can merge those changes into this branch.

Co-authored-by: Zeb Burke-Conte <zmbc@users.noreply.github.com>

RobinL · 2024-03-25T09:02:29Z

Thanks @zmbc! I've responded to your comments on the de-emphasize conda PR Happy for you to merge it in and after that we should be good to go.

Thanks again!

…nboarding-development-environment

zmbc · 2024-03-25T18:33:07Z

@ThomasHepworth I think this is just waiting for follow-up on your two unresolved comments above. Once we get those ironed out, it should be good to go!

mkdocs.yml

ThomasHepworth · 2024-03-25T19:09:13Z

@zmbc after you've tweaked the mkdocs.yml file, I'm happy to approve this from my end.

Thanks again for your work on this!

zmbc · 2024-03-25T22:08:54Z

Addressed the comments, and I believe my changes should now fix the CI failures, except that the mkdocs CI seems to not like that this PR is from a fork.

zmbc and others added 3 commits March 19, 2024 14:06

Document first-time developer setup, add conda option

f442a75

Fix broken links

542efc7

Merge branch 'master' into docs/onboarding-development-environment

877e5c2

zmbc commented Mar 19, 2024

View reviewed changes

CONTRIBUTING.md Show resolved Hide resolved

zmbc added 2 commits March 19, 2024 15:12

Tweak wording

83e3b4a

Clean up wording

4e726ea

RobinL reviewed Mar 20, 2024

View reviewed changes

RobinL assigned ThomasHepworth Mar 20, 2024

RobinL requested a review from ThomasHepworth March 20, 2024 09:23

ThomasHepworth reviewed Mar 20, 2024

View reviewed changes

RobinL added 2 commits March 20, 2024 16:23

conda suggestions

8b6ecb1

further conda suggestions

afdd9ac

RobinL mentioned this pull request Mar 20, 2024

(WIP) De-emphasise conda zmbc/splink#1

Merged

zmbc and others added 10 commits March 20, 2024 10:12

Add username and password for pgAdmin

ecaa44f

Co-authored-by: Robin Linacre <robin.linacre@digital.justice.gov.uk>

Correct typo in removal of miniforge script

cdbfd92

Co-authored-by: Robin Linacre <robin.linacre@digital.justice.gov.uk>

Make Python version guidance more generic

4c3b735

Co-authored by: Tom Hepworth <45356472+ThomasHepworth@users.noreply.github.com>

Add mamba init instructions

b4e4d74

Co-authored by: Robin Linacre <robin.linacre@digital.justice.gov.uk>

Correct mamba init path

f8d4c22

Correct check for Athena test skipping

38e7a9d

Include reason for poetry pin

c2173be

Merge branch 'master' into docs/onboarding-development-environment

a113b8c

Fix mamba installation

7c9ee85

Merge branch 'docs/onboarding-development-environment' of https://git…

fd08548

…hub.com/zmbc/splink into docs/onboarding-development-environment

RobinL and others added 2 commits March 25, 2024 08:46

Update docs/dev_guides/changing_splink/development_quickstart.md

000c812

Co-authored-by: Zeb Burke-Conte <zmbc@users.noreply.github.com>

Update docs/dev_guides/changing_splink/development_quickstart.md

41dfed7

Co-authored-by: Zeb Burke-Conte <zmbc@users.noreply.github.com>

zmbc mentioned this pull request Mar 25, 2024

Add documentation for spellchecker and spellcheck docs #2025

Merged

10 tasks

zmbc and others added 3 commits March 25, 2024 11:20

Merge remote-tracking branch 'upstream/robin_suggestions' into docs/o…

0a8845d

…nboarding-development-environment

Clean up headers, address comments in Robin's PR

e062e8c

Merge branch 'master' into docs/onboarding-development-environment

281a0e8

ThomasHepworth reviewed Mar 25, 2024

View reviewed changes

mkdocs.yml Outdated Show resolved Hide resolved

zmbc added 4 commits March 25, 2024 14:40

Fix ruff directive

c237900

Rename Poetry page

b45a8c4

Lint

e0250b8

Rename dependency policy page

68bd5d3

RobinL approved these changes Mar 26, 2024

View reviewed changes

RobinL merged commit 2e7eda3 into moj-analytical-services:master Mar 26, 2024
8 of 9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Document first-time developer setup, add conda option #2083

Document first-time developer setup, add conda option #2083

zmbc commented Mar 19, 2024

RobinL left a comment •

edited

Loading

RobinL Mar 20, 2024

zmbc Mar 20, 2024

RobinL Mar 25, 2024

zmbc Mar 25, 2024

RobinL commented Mar 20, 2024

ThomasHepworth commented Mar 20, 2024

ThomasHepworth left a comment

RobinL commented Mar 20, 2024

zmbc commented Mar 21, 2024

RobinL commented Mar 25, 2024 •

edited

Loading

zmbc commented Mar 25, 2024

ThomasHepworth commented Mar 25, 2024

zmbc commented Mar 25, 2024

Document first-time developer setup, add conda option #2083

Document first-time developer setup, add conda option #2083

Conversation

zmbc commented Mar 19, 2024

Type of PR

Is your Pull Request linked to an existing Issue or Pull Request?

Give a brief description for the solution you have provided

PR Checklist

RobinL left a comment • edited Loading

Choose a reason for hiding this comment

RobinL Mar 20, 2024

Choose a reason for hiding this comment

zmbc Mar 20, 2024

Choose a reason for hiding this comment

RobinL Mar 25, 2024

Choose a reason for hiding this comment

zmbc Mar 25, 2024

Choose a reason for hiding this comment

RobinL commented Mar 20, 2024

ThomasHepworth commented Mar 20, 2024

ThomasHepworth left a comment

Choose a reason for hiding this comment

RobinL commented Mar 20, 2024

zmbc commented Mar 21, 2024

RobinL commented Mar 25, 2024 • edited Loading

zmbc commented Mar 25, 2024

ThomasHepworth commented Mar 25, 2024

zmbc commented Mar 25, 2024

RobinL left a comment •

edited

Loading

RobinL commented Mar 25, 2024 •

edited

Loading