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

Fix 6.26.0 header level in CHANGES #394

Closed
wants to merge 1 commit into from

Conversation

svetlyak40wt
Copy link
Contributor

No description provided.

@mosesn
Copy link
Contributor

mosesn commented Jul 15, 2015

:shipit:

@dschobel
Copy link
Contributor

🚢

@taylorleese
Copy link
Contributor

@dschobel Not to be confused with 🚢 🇮🇹 or 🐑 🇮🇹.

@svetlyak40wt
Copy link
Contributor Author

Nice. Does somebody have rights to merge this pull request? :)

@svetlyak40wt
Copy link
Contributor Author

Without this fix, allmychanges have problems which parsing changelog as reStructured file: https://allmychanges.com/p/scala/finagle/

@mosesn
Copy link
Contributor

mosesn commented Jul 15, 2015

Wow, I had no idea allmychanges existed, very cool! Now that someone is actually consuming the rst, it would be neat to validate that it's right. It looks fine on the site you linked to, did you fix that manually? It's correct rst, right? It's just that it's not semantically correct.

@dschobel
Copy link
Contributor

@svetlyak40wt unfortunately we're not set up to merge PRs directly via github, they have to go through twitter's internal repo which gets synced out every Monday.

details: https://github.com/twitter/finagle/blob/develop/CONTRIBUTING.md

@svetlyak40wt
Copy link
Contributor Author

@mosesn allmychanges is my pet-project. Feel free to write me any time if you have some ideas related to it.

It has few parsers, markdown, rst, html and plain-text (which is able to handle most formats, but in a very restrictive way, for example, it will not handle rst or markdown links, etc). Right now I tuned this package to use plain-text parser for finagle, because rst parser returns 6.26.0 version with empty description.

Let me explain how it works. Markdown and rst files translate texts into html and then process it with html parser :)
Html parser searches h1, h2, h3, h4, h5 headers and if there is something like version number in the text of the header, then saves elements, following this header until a header of the same or higher level will be encountered.

Right now in the CHANGES 6.26.0 version's header is followed by "Deprecations" header of the same level. That causes the problem.

Plain text parser, works differently. It just looks at each line, and if it is relatively short and has a version number, then this line is considered a version number, and all next lines are saved as description until the next version number will be found.

That it.

@svetlyak40wt
Copy link
Contributor Author

@dschobel so, I just have to wait for a Monday, right?

@dschobel
Copy link
Contributor

@svetlyak40wt if it gets merged internally this week it'll show up in the develop branch on Monday so the only variable is the bandwidth of the team to pull it in (but we're usually good about clearing out all PRs that have passed review every week).

@svetlyak40wt
Copy link
Contributor Author

Thank you, Daniel, thank you, Moses!

@vkostyukov
Copy link
Contributor

I'm pulling this internally. Will close this PR once it's merged.

@svetlyak40wt
Copy link
Contributor Author

@vkostyukov cool! Thank you!

@vkostyukov
Copy link
Contributor

This has been merged internally (and will show up here very soon). Thanks @svetlyak40wt!

@vkostyukov vkostyukov closed this Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

6 participants