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

Rethink general layout and organization. #64

Open
JJ opened this issue Aug 2, 2019 · 4 comments
Open

Rethink general layout and organization. #64

JJ opened this issue Aug 2, 2019 · 4 comments
Labels
wishlist Things that would be nice to have, but have a low priority

Comments

@JJ
Copy link

JJ commented Aug 2, 2019

Respecting the external API it needs to service, we probably need to rethink the layout and internal API so that we avoid errors such as the one told in #63

@JJ JJ added the wishlist Things that would be nice to have, but have a low priority label Nov 3, 2019
@Altai-man
Copy link
Member

Right now it's a class. It needn't be. It just uses lexical variables. It shouldn't.

It has to be, because this is the API expected by Rakudo. Calling --doc=$foo refers to Pod::To::$foo and a method render is called. This distribution is so old it actually tried to subvert that by providing a "fake" facade class and then providing a spaghetti of global variables (just say no) and subs. We most definitely want to follow OOP here, as this is the interface expected and this is what can help us with instability problems.

The core dumping referred is related to a moar issue and drifting away from global variables will help us, both class being facade and class as it is now are innocent. Most of the cleanup work for this ticket was already done in #80 and for parallel troubles we have a separate ticket, so closing.

@JJ
Copy link
Author

JJ commented Oct 16, 2020

Please don't close this. First, it's labelled *wishlist", implying a vague desire for a general rethinking which is, if course, open to comments. Second, it's quite obvious that interface must be kept, that's a given.

@Altai-man
Copy link
Member

IMHO, re-imagining the whole thing is too vague to be a working ticket.

The title states "refactor to a package", but the ticket itself says nothing about why it should be this way. In fact, it should be a class, so the title asks for something likely undesirable.

Partially, re-thinking was already done in the PR mentioned. Partially, as there are still issues like #82 that want a solution. However, such issues have a specific problem to solve, while this issue does not state what is the issue here.

Is there anything I am missing?

@Altai-man Altai-man reopened this Oct 16, 2020
@JJ JJ changed the title Refactor to a package Rethink general layout and organization. Oct 16, 2020
@JJ
Copy link
Author

JJ commented Oct 16, 2020

I just think there's too much legacy code here. I've changed the title to see if it better reflects what I think. Thanks for reopening.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wishlist Things that would be nice to have, but have a low priority
Projects
None yet
Development

No branches or pull requests

2 participants