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

Add more code comments #2223

Closed
mikunimaru opened this issue Nov 21, 2023 · 6 comments
Closed

Add more code comments #2223

mikunimaru opened this issue Nov 21, 2023 · 6 comments

Comments

@mikunimaru
Copy link

Arch Installer is a great script.
Given how skilled Arch users are, I think there are a lot of potential users who can actually contribute code, not just submit issues.
However, the current code is poorly commented and difficult to contribute without reading deeply.

I would like to propose making it easier for arch users to contribute by writing code comments that are too polite.

Specifically, the idea is to broadly accept innocuous pull requests that simply add explanations to the code. This is also consistent with the traditional way Arch's community contributes.
Additionally, as more users read the code and contribute comments, potential bugs have a greater chance of being discovered.

@svartkanin
Copy link
Collaborator

I'm not sure I fully understand the proposal, are you saying you want to add comments for logic or doc strings for functions explaining parameters and usages of interfaces?
On the former one, I'd probably argue that any code (unless utterly complex logic) that needs explanation is not well written and would do better with refactoring as comments should not be used to explain the logic of the code.
On documenting interfaces with doc strings, I fully agree that that is lacking quite a bit. As we have rewritten and refactored quite some functionality over the past months I think it needed some stabilizing of the code base to make it actually valuable and last.

@mikunimaru
Copy link
Author

The code changes too drastically to create a separate document, so I felt it would be more realistic to write it as a code comment.
An additional benefit is that when creating documentation in the future, it will be easier for contributors to create and maintain documentation by quoting from code comments.

Most ideally, there would be a searchable comment indicating which step the process corresponds to in a manual installation.
This may be a bit too advanced, but if a user encounters trouble during installation, they can use "grep -r xxx /usr/lib/python3.xx/site-packages/archinstall" to look up the relevant code for that process. I think it would be helpful for advanced users to have comments that they can quickly fix on their own. The user can then submit an issue with specific code to suggest a fix.

If the user wants to make a personal change that is clearly not suitable for a pull request, the user will need to modify the code themselves, but it is also convenient to be able to easily inspect the code in the script. (For example, if the user wants to change only the size of the EFI partition during guided installation.)
It also makes it easier to understand what is going on with Arch Linux internally, so if a problem is discovered later, it will be easier to understand which part to fix.

@mikunimaru
Copy link
Author

In other words, there are still too many changes to create documentation, but I felt that there were too many users for no documentation, so as a compromise, if sentences that are similar to documentation are written as comments in the code. If so, wouldn't it be possible to provide users with information similar to documents while maintaining some level of consistency?

Until the code is stable, the documentation can be replaced with the text "Please see code comments for details."
I think that's enough for Arch Linux users.
If anything, it's kinder to help users understand what it's doing by looking at the code itself rather than just the documentation.

@svartkanin
Copy link
Collaborator

There is an ongoing pr from @Torxed to update docs #1967

@Torxed
Copy link
Member

Torxed commented Nov 21, 2023

I agree with docstrings being the way forward.
Functions should be small enough, and code simple enough that — with doc-strings it should paint enough of a picture for everyone to participate.

I'll keep this one open until #1967 is merged, I consider that PR to address this.

Edit: I will add that, if anyone wants to participate but the above is not enough - I think a better way forward is to just hit me up and I will be happy to sit down and do a 1:1 and bounce ideas/issues/knowledge-share. Just be respectful/mindful of time-zone differences and that life some times have a tendency to get the best of everyone at some point. :)

@svartkanin
Copy link
Collaborator

As #1967 was merged I think we can close this

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

3 participants