@qingfeng.xia: I want to help you with the code, but I ask for something in return.
1. Try to split your code into small commits. Adding test files -> separate commit. Adding icons -> separate commit. Renaming a_funcion into b_function -> separate commit. Renaming file -> separate commit. "git reset" and "git add -p" are really good for it.
2. Please use good commit messages. "using git commit -a to make sure all modified files are commit" is great in your personal repository, but for me it's _wrong_ if I have to do anything with that commit. I tried to split your code into small logical chunks and I had to start with squashing all your changes into one big commit because of that type of commits.
3. Adding comments like in your personal repo is OK, but for a reviewer it's just a distraction
Code: Select all
- def IsActive(self):
+ def IsActive(self): # bad naming, need reconsidering
4. There are a lot of that kind of functions in the code:
Code: Select all
def write_solver_control(self):
pass
It's not any good for the person reviewing the code. Just skip the function completely including the call of the function. Add a comment that something should be added in the future, but do not add a mock function.
5. Please use file name corresponding the python class name.
Code: Select all
$ grep "class.*:" src/Mod/Fem/ccxFemSolver.py
class CaeSolver():
class ViewProviderCaeSolver:
class _SolverControlTaskPanel:
fedora:/home/przemo/software/FreeCAD/freecad
$ grep "class.*:" src/Mod/Fem/CaeSolver.py
class _CommandNewCaeSolver:
File CaeSolver doesn't contain CaeSolver class, but CaeSolver class is defined in ccxFemSolver.py?
6. flake8 throws syntax error on src/Mod/Fem/FoamCaseWriter.py in line 73. Please make sure that the code is correct at the minimum.
7. Merging master on top of your changes doesn't help with anything. Please use "git rebase master" on you feature branch (master is the FreeCAD master for me)
