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

Time Commitment Grant Proposal: Refactoring Sketcher #6

Open
AjinkyaDahale opened this issue Mar 30, 2024 · 8 comments
Open

Time Commitment Grant Proposal: Refactoring Sketcher #6

AjinkyaDahale opened this issue Mar 30, 2024 · 8 comments
Labels
funded The FPA voted to fund this proposal

Comments

@AjinkyaDahale
Copy link

AjinkyaDahale commented Mar 30, 2024

Proposal description

This project is about making Sketcher more manageable to maintain and extend. Owing to the vast types of curves, and cases within cases that are generated, almost every method and command extends to multiple hundred lines, with some files going beyond 10,000 lines. Even with attempts to handle these cases, problems still arise, as can be seen in the list of Sketcher issues on the repository. Given how much this WB is used, any corner case left behind is eventually bound to surface, and with every added feature and curve type, the number of these corner cases is only going to increase.

Thus, I propose to take a step back before adding any further changes, and refactor the codebase for better readability and maintainability.

Deliverables

  • Simplifying methods: Most of the methods in Sketcher are gigantic with multiple cases for various supported geometries. Often, these cases are repetitions or slight modifications of previous cases. These can be moved to separate helper methods for reuse, or at least divided into pieces for each case, making an easier reading.
    • The largest files include src/Mod/Sketcher/App/Sketch.cpp, src/Mod/Sketcher/App/SketchObject.cpp, src/Mod/Sketcher/App/planegcs/GCS.cpp, and src/Mod/Sketcher/Gui/CommandConstraints.cpp, and as a heuristic, these would be first targeted.
    • Cognitive complexity of these (and possibly other) files may be used as a metric. In CommandConstraints.cpp alone, there are 18 cases where the currently accepted threshold of 25 is breached, with one method activated goes as far as 138.
  • Testing: Sketcher currently lacks in terms of testing. This makes it susceptible to regressions, especially with the amount of changes being added to it. Before any major refactoring, sufficient tests will be added.
  • Documentation: This would include both
    • a complete documentation of methods, especially if they are user facing, and/or have corner cases that need to be clearly stated.
    • a set of "best practices" to follow while performing such refactoring elsewhere.

Timeline

Work can start as soon as funding is approved, and can be divided as follows:

  • Month 1: Learn more about refactoring and plan next actions.
  • Month 2-4: Add tests, change code, repeat.

I would be dedicating about 10 hours per week for this project on an average.

Risks and mitigation

Given the nature of the task, I cannot for sure say how much work is involved here. It may just involve dividing the files and methods into manageable chunks, or may go as far as rewriting them in their entirety. That is why I am proposing this as a time-commitment grant.

A risk on the personal front is that I may join a full-time job in the middle of this effort, if the right opportunity presents itself. In that case, I would like to continue this project in my free time as much as possible (with compensation negotiated accordingly), or at least wrap it up beforehand if I am restricted from doing so.

Compensation

I would like a compensation of at least 5,000 USD, paid in installments of 1,250 USD per month, or the remainder of the amount if the project is completed soon enough.

About you

My name is Ajinkya Dahale. I go by jnxd on the FreeCAD forum and AjinkyaDahale on Github. I am an independent software developer based in India, and a contributor to FreeCAD since 2016.

Experience relevant to the proposed grant: Most of my contributions to FreeCAD have been in Sketcher, particularly in adding B-spline support. This has provided me the opportunity to delve into various parts of Sketcher, including the planegcs solver, and both the command line and GUI interfaces.

I have always aimed to keep my changes readable and compact enough (or have been pushed to do so by the maintainers). This is demonstrated best in this PR to support splitting more curves, where I had to generalize a lot of the code to support double the number of types of curves without doubling the size of the split method. That being said, solely refactoring will be fairly new to me, and I would be "learning on the job".

@chennes
Copy link
Member

chennes commented Apr 3, 2024

Thank you for the grant proposal, @AjinkyaDahale -- the FPA grant review committee has begun reviewing grants and is requesting changes to this proposal.

  1. Please only include the base proposal: any extension will be considered as a separate grant, rather than an optional addition to this one.
  2. Please pre-select a subset of Sketcher files that you definitely plan on working on (you may choose any you like) so that the scope of the work (and the review) can be narrowed.

@AjinkyaDahale
Copy link
Author

Thank you for the grant proposal, @AjinkyaDahale -- the FPA grant review committee has begun reviewing grants and is requesting changes to this proposal.

1. Please only include the base proposal: any extension will be considered as a separate grant, rather than an optional addition to this one.

OK. How do I apply those changes? As an edit to the original post, or as a comment?

@chennes
Copy link
Member

chennes commented Apr 4, 2024

As an edit, please.

@yorikvanhavre
Copy link
Member

I would also like to see a better plan of what you want to achieve. Even being a time commitment, this is still a project, it should have a goal

@chennes
Copy link
Member

chennes commented Apr 9, 2024

I am noting for the record that because of the delay due to @AjinkyaDahale's travels, the Grant Review Committee is holding off on its review of this proposal until he has time to respond. A decision about funding this grant will not be completed by the original timeline of the grant process.

@chennes chennes added the on hold This proposal is not currently being considered, and is waiting for something label Apr 16, 2024
@AjinkyaDahale
Copy link
Author

Thanks for putting it on hold, @chennes. I have updated the proposal based on the feedback.

@chennes chennes removed the on hold This proposal is not currently being considered, and is waiting for something label Apr 30, 2024
@chennes
Copy link
Member

chennes commented Jun 3, 2024

This grant has been approved by the FPA. Comments from the grant review committee:

The Grant Review Committee supports funding this grant. We view this is a ‘proof-of-concept’ pilot project, affecting a small and concentrated part of the codebase, but one that needs significant work to bring up to modern best-practices. That work is often thankless and hidden from view, so it is difficult to find volunteer developers to work on projects of this sort. Ajinkya is familiar with the codebase, has already proposed several methods that greatly exceed recommended cognitive complexity levels to begin his focus on, and intends to document the process so that other developers can follow in his footsteps, both in Sketcher and beyond. Finally, this is an opportunity for a regular contributor to FreeCAD to improve his own software development skills, which is something that will pay dividends in the long term.

Congratulations, @AjinkyaDahale -- please contact [email protected] with your payment details and expected starting date.

@chennes
Copy link
Member

chennes commented Jun 3, 2024

(Pinging @prokoudine FYI)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
funded The FPA voted to fund this proposal
Projects
None yet
Development

No branches or pull requests

3 participants