-
Notifications
You must be signed in to change notification settings - Fork 871
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
Extension Api doc #946
Extension Api doc #946
Conversation
Looks great so far. I added the [work in progress] label to this. When you are finished, feel free to remove the label. |
Hmm.. The spell checker is causing more problems than it is fixing. It flagged 'encodings' and cannot see that table entries should not be checked. |
If you find a legitimate word which is not in the spelling dictionary, you can add it to .spell-dict (one word per line). However, I find that is rarely necessary. Usually the problem is that an abbreviated form is being used (when the full form should be used) or the word should be in a code span. |
Well; it passes spelling and another check gets run. :) I hope to get out of the house today and hike up a mountain; it may stay broken for some time. |
Coverage failure can be ignored as nothing you've touched is code. |
OK. This completes the Phases of Processing section. I would like to integrate this now. There will always be more to write. |
Approve? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started reviewing this, but with the order of section changed around, its very difficult to see what has changed and what has simply been moved. Also, the formatting of some sections seems to have been completely reworked (definition lists removed). I'll need to look at the before and after in their rendered forms to properly review this. And with my limited availability, that will be some time from now.
However, I have previously raised some concerns which seem to either have been ignored or brushed off as not important. I even raised them early on, but the rest of the document has been edited as if they were never mentioned. Sorry, but this is a no-go without those concerns addressed.
Finally, In the early sections of the document, as I began my review, I pointed out every little issue (an extra word hear, a missing word there). As I got further in, I paid less attention to those things. At first my thinking was that If that all there was, when I was finished I would go through and make those changes myself. But, now I much bigger concerns that need addressed first.
Before I put in more work, is the answer simply "this will never be approved?" There are many options on a project, and simply keeping it static, without changes is an option. As this document has only had three small commits in the last few years, it may be a project in stasis. Are you expecting a path towards acceptance or are you choosing stasis. While the choice of 'throw out the long document because new Python programmers might not understand |
From your last comment I get the impression that you expect us to just wholesale accept your large change. I realize that many projects tend to not scrutinize larger changes closely, but I am not that sort of person. A larger change should receive more scrutiny. In fact, your work is full of minor typos. It requires a lot of work before it will be accepted. Given the large amount of time that will take and the fact that the few concerns which were previously raised were ignored, I am not inclined to spend the time to review if you are just going to ignore my review. However, if you are willing to take my review seriously and make the adjustments, then I am happy to work through a review with you. In other words, if you address all of the concerns I have already raised, then I will take the time to continue my review and get this into an acceptable condition. As an aside, you seem to be really stuck on the |
I should add there there are some good changes in here. Regardless of what you do going forward @merriam, we could very well use much of this work. Its just that my time to spend on this is very limited right now. So it may be awhile. |
Thank you for your quick response. I understand it is a lot to review and I will try to address each of your issues. I will submit updates to this PR for points you bring up and keep things clean by not submitting any other PRs until this one is done (though #897 is on my short list). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made a bunch of updates to this. I have mostly broken my changes up into commits by category. Because one of the changes included re-wrapping almost every line in the document, it may be best to review each commit separately to see what I did.
docs/extensions/api.md
Outdated
<hr> | ||
|
||
<p>You could create zombies by mixing lime and coconut.</p> | ||
<div style="display: inline-block; border: 1px solid red;"> | ||
<p>Never do that!</p> | ||
<p>Everyone might <strong>die</strong>!</p> | ||
</div> | ||
<p>Let's not.</p> | ||
|
||
<hr> | ||
!!! note "" | ||
<p>A regular paragraph of text.</p> | ||
<div style="display: inline-block; border: 1px solid red;"> | ||
<p>First paragraph of wrapped text.</p> | ||
<p>Second Paragraph of **wrapped** text.</p> | ||
</div> | ||
<p>Another regular paragraph of text.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I just realized I commented on a different example than the one I screenshot. Anyway, the point remains.
Version 3.2 has already been released. Therefore, we would not add more changes there. Instead, this goes under the version under development: 3.2.2. Bugfix releases simply get their notes in the chnage_log, not their own seperate page.
@merriam thanks for spearheading this. That was a lot of much-needed work. |
Updating the API docs as I write an extension.
Work in progress