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

Display add-on docs using marked #54

Closed
wants to merge 1 commit into from

Conversation

3 participants
@pro-src
Copy link
Contributor

commented Jun 5, 2018

WIP - don't merge

This is complete in terms of concept and required code.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented on _includes/docs-category-article-list.html in db4e0b6 Jun 5, 2018

This should be endunless instead of endif but I'm just prototyping the idea. I'll push another commit to remove any kinks if this gets a +1

This comment has been minimized.

Copy link
Member

replied Jun 6, 2018

FYI if you do a review comment (via the PR files tab) instead of commenting on the makes it a little easier as a reviewer (plus I get less notifications 😄)

@Tyriar
Copy link
Member

left a comment

I'm really starting to lean more towards just having a list of recommended addons. We do away with the complexity of fetching and everything, which means less chance of bugs and less code to maintain. Plus it's just a list so it's easy to add both official and non-official addons.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Sigh...

Why do you see those 66 lines of simple client side code needing to be maintained?

There's nothing really complicated about it but if your against it, just say so and I'll stop waisting my time.

As far as notifications go, everybody that is subscribed to xterm.js very recently got like a 100 of them in just a couple of days from you, Lol. Nothing is requiring you to respond promptly.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

@joshbruce I'm really interested to know how often the code for this page https://marked.js.org/#/README.md#README.md needs to be updated if ever at all?

Is the code there overly complex, subject to breakage, or would you recommend it to others for production?

@parisk

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2018

@pro-src let's tone this down a little bit. No one is wasting your time here — based on what I see you opened this Pull Request voluntarily.

Xterm.js and all related projects are being maintained by volunteers. This means that we put time to review and evaluate contributions, including yours, for which we won't get paid, ever. The least we expect for this is respect and politeness towards our expressed opinions, even if you do not agree with them.

If we say that a piece of code is convoluted and hard to maintain, this is enough. We can definitely have a polite conversation around this, but no one should expect other developers, including of maintainers of libraries that we use, just saying "it's not complicated" should affect our perception of complexity and maintainability.

Let's move on to the technical side of this PR now.


@pro-src what is the problem that you are trying to solve with this PR? Why use marked to display add-on documentation? Seems to work pretty fine right now (e.g. https://xtermjs.org/docs/api/addons/attach/).

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Of course my time is being wasted when I spend it to make contributions that are repeatedly shot down. If it's reasonable for something not to be approved then I at least expect to be informed of those reasons. Just saying that it's complicated is better than saying it's simple?

This conversation started on another PR and the tone certainly isn't correct, neither is the review. I've been making contribution after contribution. Are you suggesting that I'm getting paid? Of course this PR was opened voluntarily but due to the lack of respect towards it, do you really expect me to open another?

The very fact that your asking what this PR aims to solve shows the lack of importance placed on my very recent contributions which have generally been about solving the same problem.

I'll move on to a more reasonable open source project now. One that isn't so hypocritical and puffed up.

@parisk

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2018

I am sad to see the tension keeping up. I do not get it.

If it's reasonable for something not to be approved then I at least expect to be informed of those reasons. Just saying that it's complicated is better than saying it's simple?

Yes, let me elaborate. One of our maintainers reviewed to this PR politely and believes that the proposed solution is more complex than just listing add-ons.

Why is this a stronger argument that yours? Because we are the ones who are responsible for this piece of code and the ones expected to deal bug reports etc. that may come up after it gets merged. As long as code exists, bugs exist as well. We just choose what kind of bugs we want to spend our time on as volunteers.

Are you suggesting that I'm getting paid?

Absolutely not. We respect your contributions and we are sincerely thankful for the time you put to make our project better. This is visible in the reviewed and merged PRs of yours so far; #44, #46, #49, #50, #55 and #56.

Of course this PR was opened voluntarily but due to the lack of respect towards it, do you really expect me to open another?

We have no say on where you choose to put your personal time. We do not expect from any one particularly to open PRs to our projects. We welcome contributions and are always happy to review equally proposed changes from anyone in the world.

Also, saying "no" to a contribution is not lack of respect. We get to chose what gets in our repositories. As long as we communicate our intentions politely, I do not see why this should be considered lack of respect.

I'll move on to a more reasonable open source project now. One that isn't so hypocritical and puffed up.

That's totally up to you, although we are sad to see insulting and hostile behavior towards ourselves.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Yes, let me elaborate. One of our maintainers reviewed to this PR politely and believes that the proposed solution is more complex than just listing add-ons.

#47 isn't too complex? Your comment would make sense if comments there didn't imply approval. I raised the complexity issue myself and this PR is way simpler.

We respect your contributions and we are sincerely thankful for the time you put to make our project better. This is visible in the reviewed and merged PRs of yours so far; #44, #46, #49, #50, #55 and #56.

Yes, my contributions are visible. It's also visible in issues where other more considerable code exists but I don't see the need to link them all to this conversation. Unless your saying that you memorized those numbers... Lol, in that case, I'm impressed.

That's totally up to you, although we are sad to see insulting and hostile behavior towards ourselves.

Overall, I do sense a bit of sincerity but not enough for me to really care. Don't be sad. Let's accept the truth and move on.

@parisk

This comment has been minimized.

Copy link
Contributor

commented Jun 6, 2018

Wrapping this up.

I did not review this PR or #47 and did not express a personal opinion about the complexity of your code.

I stepped into the discussion, when I saw hostile and insulting tone towards the maintainers, mainly when you claimed we are wasting your time in #54 (comment) and later when you claimed hypocrisy from our behalf in #54 (comment).

All I am advocating is respect and politeness.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Wow, somebody lock this already!

I said that I was wasting my time in that comment which makes perfect sense if my PR isn't actually being considered.

Secondly, there was nothing insulting about that comment. Those notifications were very real but I apologize if pointing that little detail out is somehow insulting.

Lastly, I believe you stepped in because @Tyriar didn't have a good counter argument but whatev. Feel free to construe whatever you like.

Wrapping this up...Quick let me be pretentious. I'm not bias at all. I just want politeness.

Please realize that disagreement isn't necessarily impolite. Although, I admit that I wasn't trying to be polite. I'm just calling it as I see it and I see this as a joke.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

@parisk I thought you were a cool dude all the way up until you exaggerated my initial tone to justify your comments. Even putting words in my mouth. You're definitely not honest if you're willing to paint a deceptive picture to justify yourself and/or demonize an ex-contributor. Thus I concede, you should be sad.

FYI, had Tyriar replied with a definitive answer or simple closed the PR then I would've quite simply let it go. It's your comments that were the last straw.

@Tyriar

This comment has been minimized.

Copy link
Member

commented Jun 6, 2018

@pro-src

#47 isn't too complex? Your comment would make sense if comments there didn't imply approval. I raised the complexity issue myself and this PR is way simpler.

In #47 (comment) we were still discussing, I proposed the super simple solution and you rejected it and created the PR. You're talking about politeness but that's exactly what this was; a gentle suggestion and start of a new discussion.

On the topic of complexity, any code that is introduced causes complexity which has maintenance burden. When issues are created or topics are discussed, ideal solutions to the problems may not have been formulated yet (as that takes time). I came to the realization over time that the ideal solution given where addons are going (xtermjs/xterm.js#1128) is to not even bother integrating the addons into the website, a simple link will point at the most up to date README.

Lastly, I believe you stepped in because @Tyriar didn't have a good counter argument but whatev. Feel free to construe whatever you like.

I didn't comment as I was busy and hadn't read it yet.

As far as notifications go, everybody that is subscribed to xterm.js very recently got like a 100 of them in just a couple of days from you, Lol. Nothing is requiring you to respond promptly.

This was just a suggestion. The normal way you would do this is add a comment on the review, that way the code diff shows up nicely in the PR and people don't get 2 notifications popping up in their inbox instead of the one.

On the notifications that I caused, I closed off 60+ issues and am still working through this and I'm still not done. This is necessary housekeeping for the project.

I'll move on to a more reasonable open source project now. One that isn't so hypocritical and puffed up.

Feel free. As @parisk said, we welcome contributions but it's your time and you can do whatever you want with it.

I'm closing this off as I think we've found the ideal solution now.

@Tyriar Tyriar closed this Jun 6, 2018

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

You're talking about politeness but that's exactly what this was; a gentle suggestion and start of a new discussion.

I wasn't talking about politeness persay. I said that I wasn't trying to be polite but that's not what you're implying. @parisk was the one talking about politeness.

I was mocking him in this comment...

Wrapping this up...Quick let me be pretentious. I'm not bias at all. I just want politeness.

...I think that to be rather clear.

On the topic of complexity, any code that is introduced causes complexity which has maintenance burden.

Really now, are we talking about xterm.js or simple client side code because context matters here. In reality you'd only ever have to delete a revision of that code when it doesn't make sense to have it anymore or do a version bump of marked. And I'd imagine even that to be unnecessary over the course of years. Unless you were you expecting it to be there 10 years from now. If you said that to appeal to the people then who doesn't like a more often true then not philosophical comment? But likely your just confused. It's okay.

I came to the realization over time that the ideal solution given where addons are going (xtermjs/xterm.js#1128) is to not even bother integrating the addons into the website, a simple link will point at the most up to date README.

I don't have a problem with that aside from the discussion not being clear about whether my PR was being considered. Its still not clear whether it was being considered at the time that I asked you to let me know if you're against it. You're clearly against it and that's fine.

This was just a suggestion. The normal way you would do this is add a comment on the review, that way the code diff shows up nicely in the PR and people don't get 2 notifications popping up in their inbox instead of the one.

Excuse me for being a bit annoyed at the comment/notification correction. Again, context matters here. In the context of all the recent notifications, you could have let that slide.

On the notifications that I caused, I closed off 60+ issues and am still working through this and I'm still not done. This is necessary housekeeping for the project.

Yup, that's pretty obvious.

@pro-src

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.