Updating contributing doc to include dev workflow improvements #1639
Draft
rma-bryson wants to merge 4 commits intodevelopfrom
Draft
Updating contributing doc to include dev workflow improvements #1639rma-bryson wants to merge 4 commits intodevelopfrom
rma-bryson wants to merge 4 commits intodevelopfrom
Conversation
CONTRIBUTING.md
Outdated
| a. NOTE: within reason. Location names, absolutely, but otherwise make sure the purpose of the name is clear. | ||
| 8. Name files consistent with the purpose of the test. | ||
| 9. Narrative tests: Test method names should be a narrative summary of the test. For example, `test_stream_create_then_update_then_delete_success(...)`. | ||
| a. Use underscores to make the narrative structure of the test name clear. |
Collaborator
There was a problem hiding this comment.
when looking at the a, b, c view in rendered they arent on new lines
Collaborator
Author
There was a problem hiding this comment.
fixed. Also fixed the a,b in item 1
…luding openApi swagger ui screenshots and test-method-naming-strategy.
rma-bryson
force-pushed
the
feature/document-dev-workflow-improvements
branch
from
8bc5df6 to
9697d61
Compare
rma-psmorris
suggested changes
Mar 13, 2026
CONTRIBUTING.md
Outdated
| 8. Name files consistent with the purpose of the test. | ||
| 9. Narrative tests: Test method names should be a narrative summary of the test. For example, `test_stream_create_then_update_then_delete_success(...)`. <br> | ||
| a. Use underscores to make the narrative structure of the test name clear.<br> | ||
| b. The method name should be a clear summary of what is being tested (create, get, update, delete) in order of operations.<br> |
Collaborator
There was a problem hiding this comment.
note how the example follows this with then
CONTRIBUTING.md
Outdated
| a. Use underscores to make the narrative structure of the test name clear.<br> | ||
| b. The method name should be a clear summary of what is being tested (create, get, update, delete) in order of operations.<br> | ||
| c. It should also include the expected result (success, error code, etc.)<br> | ||
| d. Comments should also be used in narrative flow to clarify the details of the test internally, but not as a substitute for a clear method name.<br> |
Collaborator
There was a problem hiding this comment.
can you provide an example for comments that follows this rule?
Collaborator
Author
There was a problem hiding this comment.
added link to example
CONTRIBUTING.md
Outdated
| c. It should also include the expected result (success, error code, etc.)<br> | ||
| d. Comments should also be used in narrative flow to clarify the details of the test internally, but not as a substitute for a clear method name.<br> | ||
| e. Setup and tear down methods should also be clearly labelled as such. Setup/teardown methods should call into well-named helper methods to clarify the purpose of the setup/teardown steps.<br> | ||
| (example: setup method calls helper method `create_test_location()` followed by `create_test_time_series()` to clarify the purpose of the setup step)<br> |
Collaborator
There was a problem hiding this comment.
you can also add in the corresponding tear downs for the location and time series.
Collaborator
Author
There was a problem hiding this comment.
added corresponding tear down example
| Please submit all PRs to the develop branch. | ||
|
|
||
| If there is an OpenAPI change, the pull request should include a screenshot of the Swagger UI showing the change | ||
| under the docs/swagger-screenshots directory (for example, see [Locations.png](https://github.com/USACE/cwms-data-api/blob/abfa3d8cd6cd9cd79c8435761fa95c447e534afc/docs/source/swagger-screenshots/Locations.png)). |
Collaborator
There was a problem hiding this comment.
can the image be resized and shown inline?
Collaborator
Author
There was a problem hiding this comment.
probably, my intent here was to link to the directory where we can insert screenshots for future PRs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
highlights including openApi swagger ui screenshots and test-method-naming-strategy.