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

Add user group stats page, user stats page, and logged-in user homepage ADRs #5316

Merged
merged 6 commits into from
Oct 5, 2023
Merged

Add user group stats page, user stats page, and logged-in user homepage ADRs #5316

merged 6 commits into from
Oct 5, 2023

Conversation

mcbouslog
Copy link
Contributor

Package

  • docs

Describe your changes

  • add ADR 51: user group stats page
  • add ADR 52: user stats page
  • add ADR 53: logged-in user homepage

How to Review

Helpful explanations that will make your reviewer happy:

  • What user actions should my reviewer step through to review this PR?
  • Are there plans for follow up PR’s to further fix this bug or develop this feature?
    • ADRs for user library and root app

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate

@mcbouslog mcbouslog added the documentation Add the documentation for something label Sep 8, 2023
@mcbouslog mcbouslog requested a review from a team September 8, 2023 20:54
@coveralls
Copy link

coveralls commented Sep 8, 2023 •
edited
Loading

Coverage Status

coverage: 82.153%. remained the same when pulling 60370b6 on stats-adrs into 61804c0 on master.

@goplayoutside3 goplayoutside3 self-assigned this Sep 18, 2023
goplayoutside3
goplayoutside3 approved these changes Sep 20, 2023
View reviewed changes
Copy link
Contributor

@goplayoutside3 goplayoutside3 left a comment

Choose a reason for hiding this comment

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

Looks good! Just a couple of NB suggestions.

docs/arch/adr-51.md Outdated Show resolved Hide resolved
docs/arch/adr-53.md Outdated Show resolved Hide resolved
docs/arch/adr-51.md Outdated Show resolved Hide resolved
@github-actions github-actions bot added the approved This PR is approved for merging label Sep 20, 2023
@mcbouslog mcbouslog mentioned this pull request Sep 28, 2023
Merged
22 tasks
@goplayoutside3
Copy link
Contributor

@mcbouslog just checking in to see if this PR will be merged soon? I wrte ADR 54 about app-root in #5423

@mcbouslog @goplayoutside3
Apply suggestions from code review
d0c9e7d 
Co-authored-by: Delilah C. <23665803+goplayoutside3@users.noreply.github.com>
mcbouslog
mcbouslog commented Oct 5, 2023
View reviewed changes
docs/arch/adr-51.md Outdated Show resolved Hide resolved
@mcbouslog
Copy link
Contributor Author

@mcbouslog just checking in to see if this PR will be merged soon? I wrte ADR 54 about app-root in #5423

Yes, I'll merge today.

@mcbouslog mcbouslog merged commit 057599d into master Oct 5, 2023
7 checks passed
@mcbouslog mcbouslog deleted the stats-adrs branch October 5, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goplayoutside3 goplayoutside3 goplayoutside3 approved these changes

Assignees

@goplayoutside3 goplayoutside3

Labels
approved This PR is approved for merging documentation Add the documentation for something
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mcbouslog @coveralls @goplayoutside3