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

Documentation fixes #4241

Merged
merged 6 commits into from
Jan 14, 2022
Merged

Conversation

nbelzer
Copy link
Contributor

@nbelzer nbelzer commented Jan 13, 2022

Description
While reading the developer guides I found a few typos, a missing link, one formatting error, and a missing piece of code. In this PR I have included a fix/update for each of these (each as separate commit).

Before After
Screenshot 2022-01-13 at 11 31 09 Screenshot 2022-01-13 at 11 31 05

The missing piece fo code is defined in guides/source/developers/customizations/customizing-mailers.html.md. I believe to have updated it correctly, such that it matches the section that comes after it (which introduces the Spree::CustomOrderMailer).

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)
  • Verified that the changes are correct using middleman server

Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

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

Thanks! ❤️

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Thanks @nbelzer!

@@ -91,7 +91,7 @@ Solidus initializer:
# config/initializers/spree.rb

Spree.config do |config|
config.order_mailer_class = 'c'
Copy link
Member

Choose a reason for hiding this comment

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

😅

@kennyadsl
Copy link
Member

For some reason, the Ci didn't run. @nbelzer can you please try to force push again to see if it's triggered this time?

@nbelzer
Copy link
Contributor Author

nbelzer commented Jan 13, 2022

@kennyadsl I ran git push --force origin documentation-fixes but see no change in the CI status, is there something else I should try?

@waiting-for-dev
Copy link
Contributor

It seems CI is skipped for guides-only changes:

- run: .circleci/bin/halt-for-doc-only-changes.sh && circleci step halt || true # skip Circle for guides

@kennyadsl
Copy link
Member

🤦‍♂️ you are right @waiting-for-dev!

@waiting-for-dev waiting-for-dev merged commit 9381cb2 into solidusio:master Jan 14, 2022
@nbelzer nbelzer deleted the documentation-fixes branch January 14, 2022 07:27
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.

4 participants