Skip to content

Comments

[RF-DOCS] Update Rails Testing Guide [ci skip]#2

Open
OughtPuts wants to merge 33 commits intomainfrom
harriet-guides-testing-rails-applications
Open

[RF-DOCS] Update Rails Testing Guide [ci skip]#2
OughtPuts wants to merge 33 commits intomainfrom
harriet-guides-testing-rails-applications

Conversation

@OughtPuts
Copy link
Owner

@OughtPuts OughtPuts commented Oct 30, 2024

Motivation / Background

This Pull Request has been created to update the testing guide.

Detail

  • Section order has been changed to improve the flow of the guide.
  • Mostly, the wording has been changed for ease of understanding, brevity and readability.
  • assert_routing was added to the rails assertions.
  • Quite a few updates were made to the 'Functional Testing for Controllers' section, particularly around generating test scaffold code.

Additional information

For discussion:

  • I've seen in my original notes that I thought about splitting this guide into two - almost a 'beginners testing' and 'advanced testing' - any opinions on this would be welcome, as the changes made to this guide would stay relevant in two halves.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@OughtPuts OughtPuts changed the title Harriet [RF-DOCS] Update Rails Testing Guide [ci skip] Oct 30, 2024
Copy link
Collaborator

@bhumi1102 bhumi1102 left a comment

Choose a reason for hiding this comment

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

Reviewed up to line 500

Copy link
Collaborator

@bhumi1102 bhumi1102 left a comment

Choose a reason for hiding this comment

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

Reviewed up to line 1000 😊

Copy link
Collaborator

@bhumi1102 bhumi1102 left a comment

Choose a reason for hiding this comment

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

Finished my review. This was a long one, thanks @OughtPuts for working through it!

Copy link
Collaborator

@Ridhwana Ridhwana left a comment

Choose a reason for hiding this comment

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

This was a really long guide, and you've made some excellent updates, @OughtPuts! 💛 Thank you! ✨

I left some feedback—feel free to incorporate what you find valuable. One thing I did notice is the shift between using "we/our" and "you" throughout the guide, which felt a bit inconsistent. It seems to have carried over from the initial version.

While it's perfectly fine to use those pronouns when necessary, I find that the tone of the most of the guides (at least the newer ones) tends to lean more toward neutral language or the use of "you." I believe @bhumi1102 follows a similar approach (based on reviewing her work), but I stand to be corrected. Either way, it's a discussion worth having if we need to 😊 .

@rafaelfranca rafaelfranca force-pushed the harriet-guides-testing-rails-applications branch from e5c25db to 0adae5f Compare December 13, 2024 19:18
Co-authored-by: Petrik de Heus <petrik@deheus.net>
@rafaelfranca rafaelfranca force-pushed the harriet-guides-testing-rails-applications branch from 0adae5f to 99fa37d Compare December 13, 2024 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants