Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Allow `.yard/opts` as an alternate for `.yardopts` #344

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants

trans commented Jun 4, 2011

Having the choice to use .yard/opts instead of the .yardopts would be a nice feature so projects can be as tidy as possible --that way only one point on reference need be in a project's root, namely the .yard directory, instead of both .yard and .yardopts in cases where both are needed.

I know it must seem a tiny thing, but it means a lot to me. Hope you will accept the patch.

Allow `.yard/opts` as an alternate to `.yardopts`.
Allowing options to be placed in a file under the .yard directory
helps developers keep their project's a bit less cluttered.
Owner

lsegal commented Jun 4, 2011

A few notes:

  1. I'd prefer .yard/options if we're going to put it in its own directory.
  2. YARD creates a .yardoc directory, not .yard, how is .yard/options cleaner than .yardopts?
  3. You can't reuse the .yardoc directory, ie. .yardoc/options would not be acceptable. The .yardoc directory is often rm -rf'd by YARD when the cache is cleared, and so you should not be putting any data inside of it because it can be deleted. The contents of .yardoc should not be considered safe storage.

trans commented Jun 4, 2011

By point:

  1. I agree, .yard/options is better.
  2. If multiple options files are needed, e.g. .yardopts_guide. Also, if a yard plugin needs configuration, that can be put under .yard/ too.
  3. Yep. I understand .yardoc/ is generated. So it can't go there, which is why .yard/.

Somewhat aside, but clearly related, ~/.yard/config is per-user, but might it also be used per-project?

Owner

lsegal commented Jun 4, 2011

  1. Okay
  2. Didn't realize you were keeping up with the new "multiple yardopts" functionality. If this is the goal, then yes, it makes sense to put them inside a single dir.
  3. Okay, just checking-- I thought this might be a typo

How would ~/.yard/config be used per project?

Owner

lsegal commented Jun 4, 2011

On a sidenote, can we get some tests in this patch?

trans commented Jun 4, 2011

Well, "sure" I thought, but I just spent two hours trying to test it and it hasn't gone so well.

We should have a discussion on testing some time, I think it might be fruitful for the both of us. I am admittedly a pretty lousy tester and you are clearly very adept at it --I was pretty blown away when I opened spec/cli/yardoc_spec.rb and saw just how many tests you have written. It's very impressive. On the other hand, I'm finding some test fragility which is making it difficult for me to actually get good coverage of my patch.

In the end I made only one change to my patch, just so the current tests would actually pass. It's not really the best approach, but it does suffice. The better thing to do would be:

file = Dir.glob(options_file).first
return [] unless file

But this breaks a number of tests for which I am hesitant to touch b/c (and this is were my poor testing skills come into play) it would mean more significant changes to the tests than I feel comfortable making.

trans commented Jun 4, 2011

About ~/.yard/config on a per-project basis, I just thought that one might autoload or ignore certain plugins on a per-project basis. But after more thought it's probably a complete YAGNI, b/c it's legacy at this point, isn't it? The proper approach now being to use the --plugin option. So forget I mention it.

Owner

lsegal commented Jun 4, 2011

We can talk about how we handle tests for situations like these on IRC if you want to drop by. I'm not available today or tomorrow, but stop in on Monday and we can chat about it :)

As far as the ~/.yard/config goes, it would be interesting to have a "global options" kind of thing. That way people could specify, for instance, --yardopts .yard/config globally, and that would be the default (that way we don't need to make the lookup here more complex by hardcoding multiple globs)... but that's for another ticket.

trans commented Feb 17, 2012

Any thought/progress on this?

Reading it over again, it occurs to me that .yardoc might also migrate to .yard/doc eventually, further tidying up project directories.

Anyhow, just wanted touch base on this. I am going through all my "contrib" repos, tidying things up, and noticed this is still an outstanding pull request.

trans commented Jan 23, 2013

I see YARD itself now has three .yardopts files. Doesn't a common .yard directory seem like a good idea yet?

Owner

lsegal commented Jan 23, 2013

The amount of .yardopts files YARD has doesn't really make a case for a common .yard directory for a few reasons:

  1. Nothing is stopping us from moving our extra .yardopts files into a .yard directory. Although this would not handle the initial .yardopts file, it would remove one file from the root dir.
  2. YARD is a pretty special project when it comes to adoption of certain features in YARD. I would say it's pretty uncommon for a project to have more than one .yardopts file, so the difference for the vast majority of projects out there is still just .yardopts versus .yard/options. Frankly, when it's a single file, I think more people would appreciate the single .yardopts file rather than a directory, so I don't even thing .yard/options would be used-- YAGNI applies here. 3. The .yardopts_i18n file is actually temporary. It really should not need to exist; it only does while we build up I18n features in YARD. Having to maintain a separate .yardopts file for internationalization is a bad thing-- solving the problem of having duplication by hiding it inside of a .yard directory would be the wrong solution. The fact that we have too many .yardopts files in the root directory is a good reminder that something needs to be done to fix this issue in a more elegant way.
  3. We're not yet drowning in .yardopts files-- I strongly doubt anyone else is. If they are, point 1 applies-- nothing is stopping them from moving those extra configs into a directory.
  4. Finally, since this patch doesn't do anything to make specifying alternate yardopts files any easier, there is no functional advantage to having it, only an organizational one (that can already be implemented by projects). The patch is simply providing a new convention-- one that I don't think will be standard practice given the existence of tools that already depend on the root .yardopts file.

trans commented Jan 23, 2013

If it were only about the .yardopts files, I would fully agree. I realize the vast majority of projects only have one of these --and I am certainly not advocating that YARD stop supporting .yardopts; rather only that is also support .yard/options. Not just b/c there might be more than one .yardopts file, but b/c there is also the .yardoc directory. So there are two entries already for every project. While this pull request does not do it, that idea was eventually that .yardoc could transition to .yard/doc. So there would only even be one toplevel entry for yard, ragardless of how many other files in may have to configure it or it caches.

And sure, it is a organizational thing. I take an active effort in trying to keep project directories clean and orderly. Including submitting patches where I think it might help :-)

Owner

lsegal commented Jan 23, 2013

I am certainly not advocating that YARD stop supporting .yardopts; rather only that is also support .yard/options.

Yes, I understand that .yard/options would be in addition to .yardopts. The problem is added complexity of supporting multiple locations for the yardopts file is not necessary if people are not going to be using it.

that idea was eventually that .yardoc could transition to .yard/doc

I'm really not certain this would be the case, even if we did move to .yard/options. I have reservations about storing transient data in a subdirectory along with non-transient data. It becomes less clear what is an artifact of a YARD build versus what is configuration that should stick around. There will need to be a re-education process about updating .gitignores to hide .yard/doc instead of .yardoc. That is just not necessary.

So, while I agree that having multiple files in the root directory is technically less clean than a single directory, I don't really see this as a pressing issue. At least, I have not heard anybody complain about clutter in the root directory due to the .yard* files.

Owner

lsegal commented Jan 23, 2013

To clarify, what I am saying is that I would be happy to revisit this issue if there are more users who think this is in fact an issue that needs resolving. Until then, I'm not very comfortable making fundamental changes to the way people use YARD if it doesn't actually feel broken to the majority of those users.

Contributor

dominikh commented Jan 25, 2013

I much prefer having .yardopts and .yardoc instead of having those two in a .yard/ directory, for the reason @lsegal just mentioned. As for the number of files/directories, I still only have those two and I really consider .yardoc to be temporary data.

Moving .yardoc to .yard/doc would not just require updates to VCS ignore files, but possibly also cleanup scripts or just memorized commands (personally I just rm -rf it, I don't even know if the YARD tools have a built-in for that, and I wouldn't care for it, either)

trans commented Jan 25, 2013

Okay. I feel alright about this now as @lsegal gave me the idea to create a general purpose options config app (see dotopts). That cleans things up in a much more comprehensive manner. So I'm good. Thanks!

P.S. I just want to mention how great YARD issue handling is (and of course by extension @lsegal). Ideas are actually discussed, which often leads to new perspectives and better ways of doing things. Unlike so many other projects that just respond (and I am nicely paraphrasing), "uh, no." (If they respond at all.)

@trans trans closed this Jan 25, 2013

Owner

lsegal commented Jan 25, 2013

:) thanks @trans! Glad this was productive!

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