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

Refactor documentations and remove unnecessary parts #232

Merged
merged 1 commit into from
Mar 14, 2018
Merged

Refactor documentations and remove unnecessary parts #232

merged 1 commit into from
Mar 14, 2018

Conversation

kutsan
Copy link
Contributor

@kutsan kutsan commented Feb 21, 2018

Quick link: https://github.com/kutsan/vim-devicons

  1. Renamed common files, such as changelog.mdCHANGELOG.md as convention.
  2. Moved contents for developers in README.md to DEVELOPER.md, again as convention.
  3. Removed all link references in the bottom of file and make it all inline. Because we are using each of them only one time, creating extra complexity does nothing but sophistication for maintainability.
  4. Removed Table of Contents and Quick Links section because now we don't need that, since it's short file.
  5. Combined features and detailed features sections into one. No one needs for full list of extensions this plugin supports.
  6. Badges are now in style=flat-square format, instead of style=for-the-badge. That was because for-the-badge doesn't look modern, it's plumpy, besides it's 2018. :(
  7. Badges are now align="center".
  8. Removed Rationale section because it was useless. (Sorry, I have to say that.)
  9. Made short installation process as much as I can. Everyone knows which plugin managers they are using, showing the process for all of them is useless and bloated. I picked vim-plug as demo because it's the most popular one right now. There are tons of plugin managers out there, you can't possibly show an examples for each one of them. If users don't know how to install plugins with their plugin manager, that's their problems.
  10. Removed let g:airline_powerline_fonts = 1 section because it needs to be automatic process. You need to check if airline installed, if it's, then set that to 1, no need to say for that.
  11. Removed vimrc examples, it was useless.
  12. Removed all Extra Configuration section because there is no space for that. If they want to learn how configure, they need to use :help devicons. I added a quote for that. Please, support this approach, having ~200 lines of how to configure section in README is no good.
  13. Removed Usage section because it was useless. Plugin works (and should) out of the box.
  14. Made Contributing section more pleasant.

Created this PR because README was cluttered for newcomers. Even I sometimes struggle to find something. I made it short and readable for everyone. Some of my changes was subjective, we can talk about more to how to properly structure them.

@ryanoasis
Copy link
Owner

@kutsan Thanks for PR and it will take me a while to digest this 100%, some of these changes I am definitely for and others I am definitely against (at least at this time).

  1. 👍
  2. I have no problems with this, however the vim doc is generated directly from the readme
  3. Interesting point, not sure how I feel about it but I will think about it more
  4. Will think it over, at least could be shorter
  5. Combining does make sense, however I would not be so quick to simply say "no one needs for full list of extensions this plugin supports". Will consider
  6. So funny thing is I just adjusted those badge styles 😊 . "for-the-badge doesn't look modern, it's plumpy, besides it's 2018. :(" This is a highly subjective. I didn't follow your "it's 2018", is there some trend against flat and larger things? 🤔
  7. 👍
  8. 👍 I can see that. No worries I am not offended. It was actually higher in the readme, I think we should move it to the wiki somewhere instead.
  9. Makes sense, again I hate to lose any details that may help some beginner users. I would consider moving to the Wiki.
  10. I understand your point, but until that is the case I don't see why we should remove it. 👍 if we do add that logic
  11. 👍 Yeah, I can see it. Actually these are broken now, due to my setting up version related links.
  12. I agree it is far too long and verbose, but I would not want to lose it. Either offload to another markdown file and/or Wiki page. The only issue here would be that the vim doc is currently generated using the readme.. so adjustments would be needed for the generation too.
  13. Yes and no I suppose. In most cases yes it works out of the box as long as you get the plugin order right. There is some config that is needed for a few plugins, maybe at some point this won't be the case then we can remove yes.
  14. I can see this, I think having the "on GitHub" links was in case they were seeing this readme in other places (vim doc, vim awesome, etc.). I can see at least more of your simplification here as more pleasant yes.

Some of my changes was subjective, we can talk about more to how to properly structure them.

For sure yep, but even my ideas are subjective and if we are not open to others ideas then that would be silly 😄 . Yes I think these changes will take some talking and meeting in the middle on certain things.


Overall I see what you are getting at mainly, it is overly verbose to the point of main points being useless or not helpful and making it hard to find higher level important aspects.

I really appreciate your effort, ideas, changes. Even if we don't end up including even most of them I think this is very helpful in the long run.

Cheers

@kutsan
Copy link
Contributor Author

kutsan commented Feb 22, 2018

  1. I didn't know that but it shouldn't be like this IMO. I think README should only contain short summary about what the project is and how to get started things, meanwhile vim docs should be full-fledged detailed papers.
  2. I don't know about you but I think it makes extra difficulty. Which problem do they solve after all? Our links aren't that long and we don't use them in multiple places.
  3. Or another approch is to make them like,
    this.Table of Contents here
  4. My point was instead of showing them all, say that we are supporting all of them, if we miss something just create an issue and we'll add that. Also, I should have said like no one needs to see that in README file.
  5. I am sorry about that. 😬 Of course there is no such trend but I mean, mmm, n-nevermind. 😅
  6. Yeah, I agree.
  7. I understand your idea but I still think we shouldn't include each of them. Maybe we should create a detailed installation tutorial in wiki? So, ultimate beginners can take a look at that.
  8. Don't we just need to write?
    if exists('g:loaded_airline')
        let g:airline_powerline_fonts = 1
    endif
  9. As far as I can see, all major plugins such as fugitive, nerdtree only have those kind of contents in their vim docs.
  10. Then, let's add this logic. 😬
  11. I didn't think about that honestly. You are right! I have added that back with --amend.

Overall I see what you are getting at mainly, it is overly verbose to the point of main points being useless or not helpful and making it hard to find higher level important aspects.

I really appreciate your effort, ideas, changes. Even if we don't end up including even most of them I think this is very helpful in the long run.

Thanks! I'm very glad it's helpful for you.

By the way, I am also kind of mad about how https://github.com/ryanoasis/nerd-fonts is also verbose. 😅

@ryanoasis
Copy link
Owner

  1. Yeah that makes sense. Long readme could even be scaring some potential users away. I'll experiment this weekend with how to generate the vim doc from readme and wiki pages 🤔
  2. The only thing I think is that as long as the readme is still readable in raw format. I think perhaps only the link for the PR badge is so ugly due to the heart 😀
  3. Hmm that could be an option. I have seen that before and thought it was a cool idea
  4. I would be okay with changes as long as there is at least a list of all supported plugins on the readme, details can be offloaded to wiki.
  5. I am still a fan of the bigger style but let me think about it some more.
  6. Plugin manager setup details I think I agree can be offloaded to wiki as long as there is a link on the readme
  7. Probably, sounds like it's not necessary but to be safe I think vimdevicons should add that logic. Then we can remove
  8. I'm okay with removing if it is offloaded to wiki and there is a link to the extra config on the readme
  9. Offload to wiki 👍 There would have to still be some config documentation instead of logic: e.g. lightline because we don't want to assume setup for a plugin that has the mindset of being user configuration based
  10. I'll check it out Thanks!

By the way, I am also kind of mad about how https://github.com/ryanoasis/nerd-fonts is also verbose. 

Haha I bet! Well I try to be open to any suggestions and I will take any constructive criticism to heart.

A bug theme of offloading things from the readme to the wiki would be a good start. I've started to do that more but still a ways to go.

I've been fairly busy working on Nerd Fonts but do switch back to VimDevIcons when I can.

@kutsan
Copy link
Contributor Author

kutsan commented Feb 22, 2018

So, here is the TODO list for this PR. Feel free to edit this message.

  • 1. Rename to uppercase version of files.
  • 2. Move contents for developers in README.md to DEVELOPER.md.
  • 3. Remove link references, make it all inline.
  • 4. Make Table of Contents with <details></details>.
  • 5. Offload detailed features section to wiki. needs readme reference
  • 6. Make badges style=flat-square.
  • 7. Make badges align="center".
  • 8. Offload Rationale to wiki. (https://github.com/ryanoasis/vim-devicons/wiki/FAQ-&-Troubleshooting#rationale -- pending merge for completion)
  • 9. Offload detailed installation instructions to wiki and make a reference under README. needs readme reference
  • 10. Add logic for let g:airline_powerline_fonts = 1. Not required for the icons only for the powerline symbols. Not going to add this logic, user can set on their own
  • 11. Remove vimrc examples.
  • 12. Offload Extra Configuration to wiki and make a reference under README. needs readme reference
  • 13. Offload Usage section to wiki. needs readme reference
  • 14. Revert on GitHub link back.

@ryanoasis
Copy link
Owner

@kutsan I did recently do a logo refresh.. so you'll have to rebase or update your branch.

Can you tell me if you plan on doing offloading to the wiki? If not I will take up that task 😄

@kutsan
Copy link
Contributor Author

kutsan commented Mar 3, 2018

I updated my branch accordingly. Also, I don't have any plan about wiki, sorry.

@ryanoasis
Copy link
Owner

@kutsan Thanks. I will tackle the wiki.

@kutsan
Copy link
Contributor Author

kutsan commented Mar 13, 2018

4- Do you think it's still needed to have, since README is pretty short now? If you think it's needed, I'm going to add it.
6- I also will revert badges back to style=for-the-badge?
10- What should we do about this one?

@ryanoasis
Copy link
Owner

ryanoasis commented Mar 13, 2018

@kutsan
4 - Yeah looks better without actually. We can kill it for now (can always add it back!)
6 - For now yes can you? I will re-evaluate later possibly
10 - You don't have to mention it, I am going to add the logic Not going to add. not required, user can set. Mentioned in the wiki.

Side note:
Overall I have switched release workflow on this project to allow PR merging and faster development in general. The getting everything into a release before going to master was causing a huge bottleneck. Ref: https://github.com/ryanoasis/vim-devicons/wiki/Development-Workflow#branch-conventions

@kutsan
Copy link
Contributor Author

kutsan commented Mar 14, 2018

4- Yes, I agree.
6- I added back. Could you please check out?
10- Roger that!

Resolved conflict and now I guess we are done.

Also, thanks for invitation, I appreciate it! Although I don't know what to do with. xD

@ryanoasis
Copy link
Owner

Also, thanks for invitation, I appreciate it! Although I don't know what to do with. xD

You're welcome. Haha. I understand 😄

@ryanoasis ryanoasis merged commit 5d1f9cf into ryanoasis:master Mar 14, 2018
@kutsan
Copy link
Contributor Author

kutsan commented Mar 14, 2018

That's a lot of better now. We made a significant improvement. 😆

@ryanoasis
Copy link
Owner

@kutsan I agree, thank you for the work and ideas.

I am mostly very pleased with the results, a few things you suggested didn't make it, a few things I might tweak but overall and for now it is looking nice 😄

@ryanoasis ryanoasis added this to the v0.11.0 milestone Mar 15, 2018
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.

2 participants