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

Yard graph with alternate db fails #583

Closed
japgolly opened this issue Aug 1, 2012 · 9 comments
Closed

Yard graph with alternate db fails #583

japgolly opened this issue Aug 1, 2012 · 9 comments
Labels
Milestone

Comments

@japgolly
Copy link
Contributor

japgolly commented Aug 1, 2012

Similar to #580, yard graph --db <alternate-dir> doesn't work either.

Easily solved though. I'll submit 2 separate pull requests, one big-picture; one isolated. Choose whichever makes you happier. :)

Note though: I strongly dislike having Yard contribute two directories of purely generated data to my project root (.yardoc and doc). Other projects like to do the same and the end result is about 500 (possibly exaggerated) directories, some valuable, some transient - it all becomes very messy and strains minds and the housekeeping cost.
I therefore love that Yard supports me moving this stuff around - it just seems like the functionality hasn't been very well tested yet. THUSLY I'll likely be raising more issues around this so...
TL;DR I recommend my patch to Registry so that other pieces of Yard functionality I haven't come across yet but have a similar issue, have a chance of being fixed simultaneously.

@lsegal
Copy link
Owner

lsegal commented Aug 1, 2012

YARD doesn't generate doc unless you use yardoc and ask for HTML output (default, of course), so it's not actually generating two directories, just one.

@lsegal
Copy link
Owner

lsegal commented Aug 1, 2012

FYI approach #1 breaks the API because it changes behaviour; we can't do that. I'm pretty sure there are uses of .yardoc_file= that do not immediately load the database (like saving to a new db, for instance)

@japgolly
Copy link
Contributor Author

japgolly commented Aug 2, 2012

No worries. Is approach 2 ok then? That or we keep Registry.load where it is and explicitly call it again after setting the new path?

@lsegal
Copy link
Owner

lsegal commented Aug 2, 2012

Yea loading afterwards seems like it should be fine. If it passes all tests I can merge.

@japgolly
Copy link
Contributor Author

japgolly commented Aug 2, 2012

Sorry - the Ambiguity of the Internet strikes.
Do you mean you're happy with pull req #585? (all previously-passing tests continue to pass with it)

@lsegal
Copy link
Owner

lsegal commented Aug 2, 2012

Yes, #585 looks good as long as it's passing :) still have to test it on other environments though.

@japgolly
Copy link
Contributor Author

japgolly commented Sep 6, 2012

Referencing #589 here for clarity when you look at the pull request.

@lsegal
Copy link
Owner

lsegal commented Sep 6, 2012

I have been pretty busy for the last month, but things are winding down. I'll have a change to check this over and put out a release this weekend.

@japgolly
Copy link
Contributor Author

japgolly commented Sep 7, 2012

No worries. Pretty much the same story for me too :)

@lsegal lsegal closed this as completed in 59ed93b Oct 15, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants