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 the documentation #71

Closed
aptiko opened this issue May 24, 2024 · 7 comments
Closed

Refactor the documentation #71

aptiko opened this issue May 24, 2024 · 7 comments

Comments

@aptiko
Copy link
Contributor

aptiko commented May 24, 2024

Hello,

I want to work on this project, and the first thing I'd like to do is improve the documentation. Is this OK? I have a question already, and as I read the existing documentation (and code) carefully, I'm sure I'll have lots more questions, so I'll be asking them here.

First question: Is AbstractSegmentsContainer meant to be used by the user (i.e. the developer that uses pydifact), or only by the developer (i.e. the developer that develops pydifact itself)? My opinion (and standard practice in projects such as Django) is that if it's an internal, we should not bother the user.

@aptiko
Copy link
Contributor Author

aptiko commented May 25, 2024

As I worked, the question partly answered itself. Yes, the user needs to know about the AbstractSegmentsContainer methods and attributes that are inherited by the subclasses. (However I'm not certain he needs to know the mechanics of inheritance, such as HEADER_TAG.)

@aptiko
Copy link
Contributor Author

aptiko commented May 25, 2024

I did some work, available at my fork and compiled at a temporary place online. So far I've only worked on AbstractSegmentsContainer. Please tell me your opinion before I continue.

@nerdoc
Copy link
Member

nerdoc commented May 26, 2024

Hi @aptiko, this looks great. The docs definitely need some love. You can certwinly ioen a PR for this, and I'd be happy to merge it. Just make sure code you write is tested.
And about AbstractSegmentsContainer: yes this is meat to be used by the developer to subclass "real" SegmentsContainers, which then are meant to be used by pydifact's users. So, the AbstractSC is "internal", as the AbstractBaseUser in Django. But as long as there are some needed by users, you'd have to make subclasses as user yourself. But if you ask me, I'd make it marked internal, but document the methods and attributes, as you said.

@nerdoc
Copy link
Member

nerdoc commented May 26, 2024

Oh, and HEADER_TAG definitely isn't something the user should change. In the best case, the user should use the subclass of AbstractSegmentsContainer. So it could be named _HEADER_TAG, but this would look a bit uglier IMHO.

@nerdoc
Copy link
Member

nerdoc commented Sep 2, 2024

Hey @aptiko! How's your work going on?

@aptiko
Copy link
Contributor Author

aptiko commented Sep 4, 2024

@nerdoc Thanks for reminding me. Unfortunately it seems I won't work much on EDI, at least not at the moment. So I submitted my half-finished job.

nerdoc added a commit that referenced this issue Oct 2, 2024
Partially refactor the documentation (partial fix for #71)
@nerdoc
Copy link
Member

nerdoc commented Oct 21, 2024

closed by #74
Rest of documentation is TBD, but for now, we leave it. PRs welcome.

@nerdoc nerdoc closed this as completed Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants