openhab / openhab-docs

This repository contains the documentation for openHAB.
https://www.openhab.org/docs/
Other
268 stars 683 forks source link

Blockly: minor restructure and grammatical improvements #2263

Closed jimtng closed 2 months ago

jimtng commented 4 months ago

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

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

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.

stefan-hoehn commented 4 months ago

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.

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

jimtng commented 4 months ago

Hi Stefan,

Thanks for looking into this.

I'll try to explain a bit further here:

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 commented 4 months ago

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 commented 4 months ago

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 commented 4 months ago

sorry clicked the wrong button

jimtng commented 4 months ago

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

jimtng commented 4 months ago
  • 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 commented 4 months ago

Is Netlify disabled / broken?

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

stefan-hoehn commented 4 months ago

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 commented 4 months ago

Is Netlify disabled / broken?

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

jimtng commented 4 months ago

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

stefan-hoehn commented 4 months ago

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.

jimtng commented 4 months ago

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.

stefan-hoehn commented 4 months ago

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.

jimtng commented 4 months ago

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:

jimtng commented 4 months ago

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.

netlify[bot] commented 4 months ago

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.