Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Jadudm/improve dev docs #935

Merged
merged 7 commits into from
Apr 5, 2023
Merged

Jadudm/improve dev docs #935

merged 7 commits into from
Apr 5, 2023

Conversation

jadudm
Copy link
Contributor

@jadudm jadudm commented Apr 3, 2023

The changes in this PR are as follows:

  1. The dev docs are improved, so that someone can work straight through them and end up with a local docker stack that works and passes tests.
  2. A new requirement to the dev requirements was added, as the web container would not run correctly with out it. I alphabetized the requirements lists while I was at it.
  3. Notes about local development were moved into a separate file, as this is not a well-supported method of development on the team. If we want to keep those directions alive, we can maintain them in their own file.
  4. Added an ignore for Census .txt files in the data_distro folder, because... otherwise, they want to be committed, and they should not be.

jadudm added 5 commits April 3, 2023 14:27
@jadudm
Ignoring data load artifacts.
8dc9d51 
When someone downloads files from Census, they end up in the data_load directory.

This ignores them so they don't accidentally get committed.
@jadudm
Alphabetize requirements, add pydash to dev
b634fe9 
It's easier to find requirements if they're in a sensible order.

Needed to add pydash to the development requirements in order to get the
stack to build/run.
@jadudm
Revised docs that seem to work.
01c0561 
Will need to do a full clean test on a VM to be sure.
@jadudm
Moved local dev out of developer docs.
0378fd8 
If we're not really supporting it, it shouldn't be in the mainline docs.

And, eventually, we should consider removing this from the docs
entirely.
@jadudm
Updated docs further.
7b9a683 
Moved one or two things around so the order made sense.
@jadudm jadudm temporarily deployed to dev April 3, 2023 18:56 — with GitHub Actions Inactive
@jadudm jadudm temporarily deployed to management April 3, 2023 18:56 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023 •
edited
Loading

Terraform plan for dev

No changes. Your infrastructure matches the configuration. 
No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

✅ Plan applied in Deploy to the dev and management cloud.gov environments #12

@github-actions
Copy link
Contributor

github-actions bot commented Apr 3, 2023 •
edited
Loading

Terraform plan for management

No changes. Your infrastructure matches the configuration. 
No changes. Your infrastructure matches the configuration.

Terraform has compared your real infrastructure against your configuration
and found no differences, so no changes are needed.

✅ Plan applied in Deploy to the dev and management cloud.gov environments #12

jadudm
jadudm commented Apr 3, 2023
View reviewed changes
Copy link
Contributor Author

@jadudm jadudm left a comment

Choose a reason for hiding this comment

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

Adding the local-dev text back.

LindsayYoung
LindsayYoung reviewed Apr 3, 2023
View reviewed changes
docs/development.md Outdated Show resolved Hide resolved
LindsayYoung
LindsayYoung suggested changes Apr 3, 2023
View reviewed changes
Copy link
Contributor

@LindsayYoung LindsayYoung left a comment

Choose a reason for hiding this comment

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

I left a few extra hints as suggestions

jadudm added 2 commits April 3, 2023 16:57
@jadudm
Accepted changes, added Makefile changes.
6be223e 
Sync'd docs with Makefile.
@jadudm
Makefile includes common docker sequences
10a8e0b 
`docker-clean` and `docker-first-run` in particular.
@jadudm jadudm temporarily deployed to dev April 3, 2023 20:59 — with GitHub Actions Inactive
@jadudm jadudm temporarily deployed to management April 3, 2023 20:59 — with GitHub Actions Inactive
jadudm
jadudm commented Apr 3, 2023
View reviewed changes
Copy link
Contributor Author

@jadudm jadudm left a comment

Choose a reason for hiding this comment

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

Suggestions accepted.

jadudm
jadudm commented Apr 4, 2023
View reviewed changes
Copy link
Contributor Author

@jadudm jadudm left a comment

Choose a reason for hiding this comment

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

All changes reviewed, accepted.

@jadudm jadudm requested a review from LindsayYoung April 4, 2023 15:46
@jadudm
Copy link
Contributor Author

jadudm commented Apr 4, 2023

I literally have no idea if I have accepted or resolved the changes @LindsayYoung suggested. I tried to.

LindsayYoung
LindsayYoung approved these changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@LindsayYoung LindsayYoung left a comment

Choose a reason for hiding this comment

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

I see my suggestions

@jadudm jadudm merged commit 414e66b into main Apr 5, 2023
@jadudm jadudm deleted the jadudm/improve-dev-docs branch April 5, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LindsayYoung LindsayYoung LindsayYoung approved these changes

@danswick danswick Awaiting requested review from danswick

@tadhg-ohiggins tadhg-ohiggins Awaiting requested review from tadhg-ohiggins

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jadudm @LindsayYoung