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

[ci skip] Fix list item rendered display of 4.2 release notes. #16154

Closed
wants to merge 1 commit into from
Closed

[ci skip] Fix list item rendered display of 4.2 release notes. #16154

wants to merge 1 commit into from

Conversation

JuanitoFatas
Copy link
Contributor

@JuanitoFatas
Copy link
Contributor Author

Before this patch:

screenshot 2014-07-13 23 08 04

After this patch:

screenshot 2014-07-13 23 08 00

@chancancode
Copy link
Member

My bad! Should we just do 4 spaces everywhere in this file then? (* content for lines with a bullet point. Would that work?

@JuanitoFatas
Copy link
Contributor Author

I think we only need to change when list item has more than one paragraph. So no need to change everywhere.

@chancancode
Copy link
Member

Right I understand that it's not necessary, but would it maintain the format we want if we normalize the indentation across this file? (I think changlogs are like that)

The reason I ask is that I think it's be much easier for other people to just follow the four spam indentation than to tell everyone about the special rule. And because of the way we work on this file Im not too concerned about populating the git history and I think the benefits outweighed it.

Does that make sense? :)

@chancancode
Copy link
Member

(That's just for this one file though.)

@matthewd
Copy link
Member

@chancancode I'd suggest <sp><sp>*<sp> over *<sp><sp><sp>... but yes, I think either of those would look better than the sudden indentation changes here. Plus more likely to establish follow-able precedent, rather than just looking like a mistake.

@chancancode
Copy link
Member

👍

@JuanitoFatas
Copy link
Contributor Author

I am 👍 on this to prevent further mistakes. I am changing it.

@JuanitoFatas
Copy link
Contributor Author

@chancancode I've updated my commit. Please take a look. Do you mean this way? I checked the output looks good.

@@ -60,30 +59,30 @@ Please refer to the [Changelog][railties] for detailed changes.
### Removals

* The `rails application` command has been removed without replacement.
Copy link
Member

Choose a reason for hiding this comment

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

  * The ...
    ([Pull Request...

@chancancode
Copy link
Member

I added some examples, does it make sense? (Sorry it's a bit hard to do this from mobile)

@JuanitoFatas
Copy link
Contributor Author

Thanks for reviewing from mobile. I think it is good to go now!

@chancancode
Copy link
Member

I must have confused you even more with my comments 😄 I fixed it in 9386ddf and credited you. If you found anything else please feel free to open a new PR.

@JuanitoFatas JuanitoFatas deleted the doc/patch-2 branch July 14, 2014 03:28
@JuanitoFatas
Copy link
Contributor Author

👍 I was also confused. Thanks!

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.

None yet

3 participants