-
-
Notifications
You must be signed in to change notification settings - Fork 695
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
Blockly: minor restructure and grammatical improvements #2263
Conversation
b8bf2bd
to
0fb491a
Compare
Thanks, Jim, as always for putting so much work into it. 🥳 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
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, |
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.
Do you have a medium to do this? I'm more comfortable with text based chats, e.g. Slack / Discord / Whatsapp.
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. |
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. |
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. |
sorry clicked the wrong button |
I have renamed the file back to |
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. |
Is Netlify disabled / broken? I found #2218 and ran the preview locally. Nice!! |
I disagree at that point, Jim and I will not remove this introduction part. |
Why do you think that Netflify is broken, can you give me an example? |
Which part specifically that you wish to keep?
Usually there's a special post made by Netlify, but that doesn't exist here: |
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. If the latter was the case open an issue before and discuss the fact first and propose changes and keep topics and changes small. |
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.
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. |
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. |
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:
|
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. |
Signed-off-by: Jimmy Tanagra <[email protected]>
✅ 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).
To edit notification comments on pull requests, go to your Netlify site configuration. |
@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.