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.

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

Epic: collect needed documentation #378

Open
6 tasks
fyliu opened this issue Sep 7, 2024 · 6 comments
Open
6 tasks

Epic: collect needed documentation #378

fyliu opened this issue Sep 7, 2024 · 6 comments
Labels
epic Issue is an epic feature: docs: PD team documentation documentation on PD team processes and architecture, etc. feature: process improvement good first issue Good for newcomers role: technical writing s: PD team stakeholder: People Depot Team size: 0.25pt Can be done in 0.5-1.5 hours

Comments

@fyliu
Copy link
Member

fyliu commented Sep 7, 2024

Overview

This issue tracks the documentation that needs to be added and the issue/pr where they would've been helpful in. The intention is to make it easy to note any missing documentation without having to create the issue at the same time.

Action Items

Resources/Instructions

How to use

  1. Add a new comment for each documentation item needed using a copy of this template

    ### What's needed
    
    <!-- Description of what the missing documentation is about -->
    <!-- Draft of the documentation -->
    <!-- Link to external docs with instructions -->
    
    ### Where it's useful
    
    - <!-- Link to what prompted the need for this change/addition -->
    - <!-- Situation where this could be used -->
    
    ### Audience
    
    - <!-- Who or what role needs to perform this action -->
    
    ### Tags
    - <!-- git, django -->
    
  2. Add an action item here, linking to the comment below

  3. Create an issue to add the documentation (can be done later by another person)

    • Then, hide the original comment and replace the action item with a link to the new issue
  4. Work on the documentation issue and check off the action item (can be done later by yet another person)

@fyliu fyliu added feature: docs: PD team documentation documentation on PD team processes and architecture, etc. feature: process improvement good first issue Good for newcomers role: technical writing s: PD team stakeholder: People Depot Team size: 0.25pt Can be done in 0.5-1.5 hours labels Sep 7, 2024
@fyliu fyliu added this to the 5 - Team Workflow milestone Sep 7, 2024
@fyliu
Copy link
Member Author

fyliu commented Sep 7, 2024

What's needed

Common things to look for in a PR review

  • PR destination should be peopledepot/main
  • check model fields
    • names should be reasonable
      • FK relations should drop the ending _id when going from DB to Django notation
    • some fields already provided by AbstractBaseModel should not be added

Where it's useful

Notes

It already exists here https://github.com/hackforla/website/wiki/How-to-review-pull-requests

@fyliu
Copy link
Member Author

fyliu commented Sep 9, 2024

What's needed

Django: How to combine migrations

Say max_migration.txt is at 0027_... and you want to combine 0025-0027 into one.

  1. Roll back core app migrations to 0024

    ./script/migrate.sh core 0024
  2. Delete unwanted migration files

    rm app/core/migrations/00{25,26,27}_*
  3. Recreate max_migration.txt for core app

    docker-compose exec web python manage.py create_max_migration_files --recreate core
    # the file core/max_migration.txt should contain "0024_..."
  4. Create combined migration file for core app

    ./script/migrate.sh core

Where it's useful

  • Many initial passes for PRs have multiple migration files that should really be a single create table migration.

@fyliu
Copy link
Member Author

fyliu commented Sep 23, 2024

What's needed

How to quickly checkout a PR

Setup: add this git alias to your global git config

[alias]
    pr = !sh -c \"git fetch upstream pull/${1}/head:pr/${1} && git switch pr/${1}\"

Usage:

To check out PR #383 to a local branch called pr/383, run this.

git pr 383

Notes:

  • This works only with github, I believe.
  • The alias assumes the PRs come from a remote called upstream.

Where it's useful

  • This can be useful for all PR reviews
  • I've been using it for a while, but I thought about adding it to the docs while reviewing Create SocMajor table #383. No special reason it's that PR.

Extra info

I have another longer alias for checking out a PR to a pr/xxxx branch plus tracks the remote branch which created the PR. Here's the alias:

[alias]
    prt = "!f() { \
      git fetch upstream pull/${1}/head; \
      commit_hash=$(git rev-parse FETCH_HEAD); \
      remote_branch=$(git branch -r --contains $commit_hash | head -n 1); \
      git switch -C prt/${1} $commit_hash; \
      git branch -u $remote_branch prt/${1}; \
    }; f"

Usage:

git prt 383

Note:

  • assumption: the PR is in the upstream remote.
  • assumption: the remote containing the original working branch is set.
  • side effect: This will reset the prt/xxxx branch if it already exists.
  • check: Run git status -sb to see tracking info

@fyliu
Copy link
Member Author

fyliu commented Sep 23, 2024

What's needed

How to add git alias to the global git config

  1. Run this in the terminal to open an editor to to edit the global git config file.

    git config --global -e
  2. Add the git alias under the [alias] section and save the file.

  3. The alias is now available for use.

Where it's useful

  • When another part of the doumentation says to add a git alias, this is how.

@fyliu
Copy link
Member Author

fyliu commented Sep 26, 2024

What's needed

How to resolve migration conflict

Let's say you're rebasing and the max_migration.txt file shows a conflict. That's a sign that the numbered migration files are conflicting as well. They're named differently so their conflicts don't show up in git.

In many cases, you can resolve it using the rebase_migration command provided by django-linear-migrations.

  1. Let's say you rebase your branch to main and see a conflict in core/migrations/max_migration.txt

    git rebase upstream/main

    For this example, let's say the conflict shows that core/0025 and core/0027 are in conflict.

  2. Undo the rebase

    You'd have migrations applied that are part of the conflict. You need to roll the database back to a point before that, so you can change those migrations if necessary.

    git rebase --abort
  3. Roll back the local database to a previous migration (any number before the ones in conflict)

    ./scripts/migrate.sh core 0024
  4. Re-run the rebase

    git rebase upstream/main
    # see conflicts returned
  5. Resolve the migration conflict

    docker-compose exec web python manage.py rebase_migration core
  6. Resolve any code conflicts

  7. Run all the migrations

    ./scripts/migrate.sh core
  8. Run pre-commit checks

    pre-commit run —all-files
  9. Stage all the fixes

    git add <max_migration.txt> <changed migration files> <fixed code files>
  10. Continue rebase

    git rebase --continue

Where it's useful

@fyliu
Copy link
Member Author

fyliu commented Sep 28, 2024

What's needed

How to apply changes from one file to a different file

Draft (unfinished)

Say we have a branch that contains changes to CONTRIBUTING.md and that the content being changed was moved to another file: docs/contributing/dev_environment.md. We want to manually apply our change to the file ourselves.

  1. Rebase/Merge your feature branch to main

  2. Undo changes to CONTRIBUTING.md

git reset upstream/main -- CONTRIBUTING.md
  1. Switch to our unmerged branch with the changes
git switch our-branch-original
  1. Create a patch of the changes
git format-patch hackforla/main CONTRIBUTING.md
# only changes to CONTRIBUTING.md
# include changes since we branched off from hackforla/main
# may need to create a temporary branch where hackforla/main used to be for this command

# output:
# 0001-add-to-dev-environment.patch
  1. Switch to rebased branch without the CONTRIBUTING.md changes
git switch our-branch-rebased
  1. Apply the patch to the new file
patch -p1 docs/contributing/dev_environment.md 0001-add-to-dev-environment.patch

Where it's useful

  • When a hunk of text is moved between files, we can apply it cleanly manually, since git cannot do it automatically unless the entire file was renamed.
  • Link to PR or commit where CONTRIBUTING.md is split into several files

Audience

  • Developer who made changes to a file where the place of change is moved to a different file in main

Tags

  • git, diff, patch, moved content

@shmonks shmonks added the epic Issue is an epic label Oct 4, 2024
@shmonks shmonks changed the title Collect needed documentation Epic: collect needed documentation Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
epic Issue is an epic feature: docs: PD team documentation documentation on PD team processes and architecture, etc. feature: process improvement good first issue Good for newcomers role: technical writing s: PD team stakeholder: People Depot Team size: 0.25pt Can be done in 0.5-1.5 hours
Projects
Status: Ongoing
Development

No branches or pull requests

2 participants