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

Update Frequency Implementation #236

Merged
merged 17 commits into from Mar 7, 2023
Merged

Update Frequency Implementation #236

merged 17 commits into from Mar 7, 2023

Conversation

shannonwells
Copy link
Contributor

@shannonwells shannonwells commented Feb 22, 2023 •
edited by wilwade

Problem

problem statement, including
Closes: #219

Solution

Matched the patterns from Operations and Records and applied them to the Frequency Implementation Spec.

with @wilwade

Change summary:

  • Rewrote most of the existing Frequency Implementation Spec
  • Added Operations page
  • Added Records page

@wilwade wilwade changed the title operations first pass Update Frequency Implementation Mar 2, 2023
@wilwade wilwade requested a review from wesbiggs March 2, 2023 13:33
@wilwade wilwade marked this pull request as ready for review March 2, 2023 13:33
wesbiggs
wesbiggs requested changes Mar 2, 2023
View reviewed changes
Copy link
Member

@wesbiggs wesbiggs left a comment

Choose a reason for hiding this comment

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

Nice work. I added some suggestions but I like the structure and flow.

pages/Frequency/Identity.md Outdated Show resolved Hide resolved
pages/Frequency/Validation.md Outdated Show resolved Hide resolved
pages/Frequency/Publishing.md Outdated Show resolved Hide resolved
pages/Frequency/Records.md Outdated Show resolved Hide resolved
pages/Frequency/Records.md Outdated Show resolved Hide resolved
pages/Frequency/Identity.md Outdated Show resolved Hide resolved
pages/Frequency/Operations.md Outdated Show resolved Hide resolved
pages/Frequency/Operations.md Outdated Show resolved Hide resolved
pages/Frequency/Operations.md Outdated Show resolved Hide resolved
pages/Frequency/Operations.md Outdated Show resolved Hide resolved
@wilwade wilwade requested a review from wesbiggs March 3, 2023 15:23
@wesbiggs
Copy link
Member

wesbiggs commented Mar 3, 2023

Can you give it a readthrough for consistency in usage of () after method names? I think it's a bit of both at the moment, but not 100% sure.

wesbiggs
wesbiggs approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@wesbiggs wesbiggs 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!

wilwade
wilwade approved these changes Mar 7, 2023
View reviewed changes
Copy link
Member

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

🚢

@shannonwells shannonwells merged commit b00fe2e into main Mar 7, 2023
1 check passed
@shannonwells shannonwells deleted the feat/update-with-frequency-operations-#219 branch March 7, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilwade wilwade wilwade approved these changes

@wesbiggs wesbiggs wesbiggs approved these changes

@JoeCap08055 JoeCap08055 JoeCap08055 approved these changes

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

Successfully merging this pull request may close these issues.

Update Frequency Implementation with Operations
4 participants
@shannonwells @wesbiggs @wilwade @JoeCap08055