openhab-docs icon indicating copy to clipboard operation
openhab-docs copied to clipboard

Blockly: minor restructure and grammatical improvements

Open jimtng opened this issue 1 year ago • 17 comments
trafficstars

@stefan-hoehn I've been working on this.

It is based on #2262, but the main changes are on the following pages (for now)

  • blockly/index.md
  • blockly/rules-blockly-getting-started.md
  • blockly/rules-blockly-date-handling.md

Please ignore the changes in all the other files for now. They are minor changes (blockly -> Blockly). The three files above are where my main changes were made.

I am glad you created the documentation in the first place and I appreciate your hard work.

What do you think of the changes? If you're happy with the idea / direction, I will continue working on the rest of the docs. It is a very time consuming work so I'd like to get your feedback first before proceeding any further.

It would be great if we had a way to generate the docs for preview purposes.

jimtng avatar Feb 29 '24 14:02 jimtng

Thanks, Jim, as always for putting so much work into it. 🥳 So nothing below is meant in an offensive way 🙏🏻 which is even why I a bit procrastinated giving this feedback 😱

In general, the more you change the more time it takes me to review it to make sure nothing gets lost or is changed in a way I wouldn't be happy of . I am a bit unclear how to proceed because it is extremely hard to understand what really has changed.

  • As you have marked it as draft, I guess (and I should know this), this hasn't even been built by netlify, so there is no easy way to see the changes as a preview (I have now checked it out locally, though, so I can review it better)
  • Only after some time I understood (correct my if I am wrong) that you renamed rules-blockly-before-using.md into rules-blockly-getting-started.md. Unfortunately, this has a major drawback because I can't review anymore what has changed. Hence, I would prefer first only to change the content and then later after a first review provide a 2nd commit that does the rename. So, would you mind to revert the rename(s) first to allow easier review for me? (only for those pages, if there are more, where the problem applies)
  • Secondly, I noticed that quite a number of images were deleted which kind of worries me because I don't understand, why.
  • Third: You did so many changes in the index.md that I fear that much of it has been thrown away and might have gotten lost. But even after looking at it I can hardly recognize the changes. So I'll have a hard time to find out what really changed. (btw, it now in the meantime has conflicts with a different change surpassing this PR)

In general, my recommendation is: if you like to provide a PR and it is a major change or contribution it is better (at least) for me if you propose something first because that can avoid frustration later on if it would be questioned by someone (like me ;-)) which is something I totally like to avoid as I am grateful for many of the contributions that come in as PRs like you! 😇

Again, don't get me wrong: you put quite some time into that but it also means I need to invest a lot of time for the review. I surely don't just wave it through, so I am kind of desperate how to move on. What I can I offer is to have a remote session together where we go through it together - maybe that is more efficient. ... and my goal is to be as quick as possible with reviews and keep the PR list very short 🥸

Many other changes that I saw are mostly english language improvements which I (not being a native speaker) of course very much appreciate

cheers, Stefan

stefan-hoehn avatar Mar 02 '24 11:03 stefan-hoehn

Hi Stefan,

Thanks for looking into this.

I'll try to explain a bit further here:

  • In index.md, I removed all the extra explanations + small image preview for each of the sub sections. The goal is to make it easier to see the actual list of subsections and read only a brief description of it in one sentence. Currently in my opinion the additional text + small images only distract readers from getting a general overview of the sections.

When they want to learn about any particular section, they can click on to it to then view the details. This separates the function of index.md vs the detailed sub-pages.

Also several of the small images that I've removed are not readable anyway because they're too small. My eyes are not as sharp as they used to these days. Unfortunate effect of aging :(

My suggestion is not to try to compare it by looking at the DIFF, but instead, view the old page and the new page side by side, as rendered / previewed pages.

It is impossible to do a makeover by only changing punctuations here and there. Sometimes the whole structure needs to be completely changed, so looking at diff wouldn't give you an understanding of the changes.

What I can I offer is to have a remote session together where we go through it together

Do you have a medium to do this? I'm more comfortable with text based chats, e.g. Slack / Discord / Whatsapp.

my goal is to be as quick as possible with reviews and keep the PR list very short

You're doing a great job at keeping the PRs under control!

This particular PR though, if I were to proceed with the rest, is going to take some time, several days / weeks. That's why I wanted to ask your feedback first before spending more time.

jimtng avatar Mar 02 '24 12:03 jimtng

I should also add.... I prefer succinct, straight to the point type of documentation. This is because it makes it so much quicker to get to the information. This is why I removed a lot of the "introduction" type paragraphs that doesn't offer any actual information about Blockly and how it works.

In my opinion, the current openHAB documentation suffers from this issue overall, not just within the blockly section.

Also some parts were moved around, e.g. the migration from oh3 to oh4 was moved towards the end of the document.

jimtng avatar Mar 02 '24 12:03 jimtng

So nothing below is meant in an offensive way 🙏🏻

I also hope that none of what I have done / written is perceived as offensive. At the end of the day, I just want to help make the best possible openhab. I understand that disagreements may arise.

jimtng avatar Mar 02 '24 12:03 jimtng

sorry clicked the wrong button

jimtng avatar Mar 02 '24 12:03 jimtng

I have renamed the file back to rules-blockly-before-using.md

jimtng avatar Mar 02 '24 16:03 jimtng

  • As you have marked it as draft, I guess (and I should know this), this hasn't even been built by netlify, so there is no easy way to see the changes as a preview (I have now checked it out locally, though, so I can review it better)

Would you like me to mark it as Ready for review, so it will be built by netlify?

I've been meaning to look into the local build process one of these days.

jimtng avatar Mar 03 '24 02:03 jimtng

Is Netlify disabled / broken?

I found #2218 and ran the preview locally. Nice!!

jimtng avatar Mar 03 '24 03:03 jimtng

I should also add.... I prefer succinct, straight to the point type of documentation. This is because it makes it so much quicker to get to the information. This is why I removed a lot of the "introduction" type paragraphs that doesn't offer any actual information about Blockly and how it works.

In my opinion, the current openHAB documentation suffers from this issue overall, not just within the blockly section.

I disagree at that point, Jim and I will not remove this introduction part.

stefan-hoehn avatar Mar 03 '24 06:03 stefan-hoehn

Is Netlify disabled / broken?

Why do you think that Netflify is broken, can you give me an example?

stefan-hoehn avatar Mar 03 '24 06:03 stefan-hoehn

I disagree at that point, Jim and I will not remove this introduction part.

Which part specifically that you wish to keep?

Is Netlify disabled / broken?

Why do you think that Netflify is broken, can you give me an example?

Usually there's a special post made by Netlify, but that doesn't exist here: #2262 #2257

jimtng avatar Mar 03 '24 06:03 jimtng

You don't have to mark it as Review to run the preview because I can review it locally. However, I won't be able to work on that before long because the changes you did are too comprehensive and differ too much from what I originally wrote. Please rather provide a PR that corrects documentation than rather delete parts that you think should be taken out. In the end this all about personal taste of documentation unless it is wrong or completely superfluous where we can always restructure text in sub pages.

If the latter was the case open an issue before and discuss the fact first and propose changes and keep topics and changes small.

stefan-hoehn avatar Mar 03 '24 06:03 stefan-hoehn

So to summarise, this PR rejected because it changes the structure too much from the original documentation? Once the documentation is written, it's set in stone and only minor typos and additions are accepted?

I'm fine with that, as long as I understand the policy, so I won't worry on suggesting any structural changes in the future.

If the latter was the case open an issue before and discuss the fact first and propose changes and keep topics and changes small.

I feel that making a PR to clearly show you exactly what I'm proposing is easier for you to see/preview rather than just describing it. Feel free to close this PR if the idea is completely rejected.

Alternatively it can be adjusted as a normal PR review process, so we're all happy with the end result.

As the custodian of the documentation, it's up to you. I can only voice my suggestions.

jimtng avatar Mar 03 '24 07:03 jimtng

So to summarise, this PR rejected because it changes the structure too much from the original documentation? Once the documentation is written, it's set in stone and only minor typos and additions are accepted?

This is not what I said, Jim, and you surely know this. This documentation is not set in stone once it is written but similar to code when you want to restructure and propose a major change (like removing functionality) I asked you to propose that change before you provide a comprehensive PR like this one.

As I mentioned before I don't agree that we should throw away the introduction information like you proposed. You, as a proficient person, may not need that but others appreciate it.

There are improvements in this PR that I would agree to, others I am not happy with.

Honestly I don't know how to accept part of the changes and others not.

Maybe we can ask others what they think, for example @Confectrician , @rkoshak, @florian-h05 and @Larsen-Locke to provide their opinion what they think about removing/restructuring what you proposed.

stefan-hoehn avatar Mar 03 '24 12:03 stefan-hoehn

This is not what I said, Jim, and you surely know this.

No, I did not know this. Perhaps I don't really understand what you're saying. There seems to be a miscommunication somewhere. I have stated my understanding upon reading your response, and at the moment I don't know how to proceed. I feel like you do not want any changes in the structure of the documentation, and only willing to accept sentence by sentence changes. That's how I understand it. This is perfectly fine of course, and that's why I haven't done all the changes I had in mind and only worked on 3 files, so if this isn't the direction you'd like, I won't need to continue.

I am also under the impression that I would need to submit individual PRs for each small change needed, which is going to be quite difficult to coordinate when the scope of the changes are bigger, some involving moving sections from one file/page to another.

As I see it, there are several options:

  • Continue and make adjustments based on your/others' feedback.
  • Abandon this PR, and perhaps in separate PRs make micro changes here and there fixing minor typos only.

jimtng avatar Mar 03 '24 13:03 jimtng

Honestly I don't know how to accept part of the changes and others not.

As I understand it, you would tell me what you want changed, I'll make the changes, and push more commits for you to review again.

jimtng avatar Mar 03 '24 13:03 jimtng

Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).

Name Link
Latest commit 55ab2af1826a12d3d57b33be5367185fc252afb1
Latest deploy log https://app.netlify.com/sites/openhab-docs-preview/deploys/6612efe3025708000894aa9f
Deploy Preview https://deploy-preview-2263--openhab-docs-preview.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] avatar Mar 04 '24 23:03 netlify[bot]