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

Fix, clean up, and clarify README #36

Closed
wants to merge 6 commits into from

Conversation

dead-claudia
Copy link
Contributor

@dead-claudia dead-claudia commented Jun 16, 2016

@MadLittleMods

Based on this comment. I've also added + fixed several other things, and I cleaned up the organization and presentation to be a little more cohesive.

What do you think?

Also, if you can think of good people to CC, to also make sure this new version doesn't read like some horrible a**hole or some random kindergartner wrote it, please do! I want this to sound cool (it does to me, anyways). 😄

Based on [this comment](MadLittleMods#4 (comment)). I've also added + fixed several other things, and I cleaned up the organization and presentation to be a little more cohesive.
We strive for the most complete transformation but we/no plugin can achieve true complete parity according to the [specification](http://dev.w3.org/csswg/css-variables/) because of the DOM cascade unknowns.
```
npm install postcss-css-variables --save-dev
```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this back to the "Install" section above the table of contents.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@MadLittleMods
Copy link
Owner

MadLittleMods commented Jun 18, 2016

Thanks @isiahmeadows, really appreciate the fresh set of eyes 😀

If you want to discuss any of these review comments in a more real-time manner, we can use the Gitter room, https://gitter.im/MadLittleMods/postcss-css-variables - While we are at it, let's add the gitter badge to the top:

[![Gitter](https://badges.gitter.im/MadLittleMods/postcss-css-variables.svg)](https://gitter.im/MadLittleMods/postcss-css-variables?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge)

@dead-claudia dead-claudia changed the title Fix, clean up, and clarify README [ci skip] Fix, clean up, and clarify README Jun 18, 2016
@dead-claudia
Copy link
Contributor Author

dead-claudia commented Jun 18, 2016

@MadLittleMods

I addressed all your comments, and I plan to rebase all the extra commits away before it gets merged. Could you please take another look?


# [Code Playground](https://madlittlemods.github.io/postcss-css-variables/playground/)
# Code Playground
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restore the link, ease of use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate further on why the link is required even though it's clearly present within the section itself? I'm not quite sold on the "ease of use" argument.

@MadLittleMods
Copy link
Owner

@isiahmeadows I did another pass, sorry for the delay 😔

@dead-claudia
Copy link
Contributor Author

@MadLittleMods I responded to most of your comments now.

I also have a few more general nits regarding how you've been looking over this. I'm trying not to offend, but it's a bit hard to avoid sounding like a serious a**hole and address these problems simultaneously.

  1. Code blocks are terrible for explaining how prose should be altered. GitHub's styles force you to scroll, and I almost always end up copying and pasting the block into the box below just so I can read it better.
  2. It's far easier for me to infer what you're getting at when you point out particular parts of sentences rather than just posting a new sentence I have to turn around and diff myself, potentially missing important information you were trying to get at.
  3. Conciseness is one thing, but it's hard to understand what you're trying to say when your responses are so frequently terse and often devoid of other content. For one, when you're too blunt and terse, it can come across as very questionable and standoff-ish. That's also why I'm not quite compressing it as far down as you have been mostly asking for ‒ I'm trying to avoid, even unintentionally, coming across the wrong way. Another reason is because it's much easier to infer the writer's intent, what they were trying to say. This goes both ways, here and in the readme itself.

I'm sorry if any of these come off the wrong way, but I don't know of any better way to explain the problem at hand.

@MadLittleMods
Copy link
Owner

MadLittleMods commented Jul 7, 2016

@isiahmeadows 👍 comments explaining how we can make this process smoother are always welcome.

I completely understand trying to phrase things as best as possible and hesitations in even bringing this kind of thing up but I have no issues discussing it. I know we both are aiming for the best outcome and reading your responses hasn't offended.

I have been posting full code snippets so that it can easily be copy pasted in place instead of trying to understand the change explained and implementing (which cuts down on the potential back and fourth of trying to figure the end goal). The code snippet allows for the markdown syntax to still be maintained but I agree it sucks to actually consume. In my case, I can just push the edit icon to see it wrapped. Sorry for not expanding on some earlier so it didn't seem so standoff-ish.

I use "we should do this ..." type language in PRs as it is a compromise between being totally terse like a commit message and asking to do something or saying please all the time.

Some of these last points we are down to are more personal preference on phrasing things instead of the obvious improvements. I'll add some better comments/thinking on what I am trying to achieve with the suggestion so we can iterate to a final solution. We could also have a Skype to bash through the remaining points quickly.

@dead-claudia
Copy link
Contributor Author

dead-claudia commented Jul 7, 2016

@MadLittleMods

What do you feel is the best time to chat on Gitter about this? (Skype/etc. won't work well for me for various reasons, including poor lighting where I live and the fact my laptop camera sucks.) I feel that might be the best way to chomp through this, since I feel we're not quite on the same page.

@MadLittleMods
Copy link
Owner

@isiahmeadows No camera needed and can be a Google hangout or talky.io room.

I am available any time during the day (CST GMT-6), https://gitter.im/MadLittleMods/postcss-css-variables

@MadLittleMods
Copy link
Owner

Thanks again for the contribution @isiahmeadows! I made a few edits and merged it in via #45 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants