Comments (10)
I'll look at this, it will help with #5 as well, under https://github.com/Stedders/AutomateGitRepoSetup/tree/Stedders.
from automategitreposetup.
Great, thanks. Please issue a PR per major change you are making.
Also I can see in your fork the code formatting is changed in some parts but please keep the original formats and only change parts of the code that you are adding functionality to.
Note, you can ignore any linting errors to do with line numbers too long.
from automategitreposetup.
Hey @funbeedev
I've gone through and reverted the changes to the main file, it is difficult to unit test the code but I have committed a change that shows how it could be done.
On the latest commit of my branch I have added a test folder with a pytest script, this excutes the main start_program_flow
function, the git workflow is setup to run pytest on every push.
I would strongly recommend refactoring the code with the following changes...
- Allows the
config_file
path to be variable and passed intostart_program_flow()
- allows us to generate configurations separate to the root directory - Don't read the configuration in the module scope - as it stands the config is loaded when the module is loaded from the running dir, this results in all variables being defined at start
- Add arguments to the functions so they are not all reading from a single object in the outer scope - allows for more specificof the functions
As it stands I won't create the PR as the code fails flake8 linting (start_program_flow()
is not in the linting scope as it is checking the code statically), this can be rectified but would need the config.ini
file in the route of the directory to be manipulated during running, which feels wrong.
from automategitreposetup.
@Stedders Thanks, I'll review your changes and suggestions and get back to you soon.
Btw, you can still issue the PR if you want even though everything might not be working. It makes it easier for me to pull your changes.
Also you can make changes to a PR after its issued by just pushing more changes to your fork!
from automategitreposetup.
@Stedders
In response to your suggestions on refactoring:
- Allows the
config_file
path to be variable and passed intostart_program_flow()
- allows us to generate configurations separate to the root directory
Is this just for more flexibility? How do you suggest the path of the config file be passed in?
- Don't read the configuration in the module scope - as it stands the config is loaded when the module is loaded from the running dir, this results in all variables being defined at start
Do you just mean to load the config within a function instead?
- Add arguments to the functions so they are not all reading from a single object in the outer scope - allows for more specificof the functions
I assume this relates to your point 2 and is about avoid use of global variables. I agree this aspect of the code needs to be improved! I'll create an issue for this.
from automategitreposetup.
Is this just for more flexibility? How do you suggest the path of the config file be passed in?
I would recommend that we add a param start_program_flow(config_file)
- If you want the path to always be config.ini then the __main__section will call
start_program_flow('config.ini')
to set it - Testing can call
start_program_flow('test-1.ini')
,start_program_flow('test-2.ini')
, etc... - In the future you could add it as an argument to the program that can be passed through
Do you just mean to load the config within a function instead?
Yes, there are a couple of options...
- Use a global that can be accessed by all functions
- Pass the config items to the functions separately
I would prefer the latter, as it allows for more granular testing - for example commit_files(msg, repo_directory)
can be tested multiple times with different messages - normal, blank, None, Japanese characters, etc... very quickly and specifically
I assume this relates to your point 2 and is about avoid use of global variables. I agree this aspect of the code needs to be improved! I'll create an issue for this.
from automategitreposetup.
@Stedders
Sorry the delay in responding.
Those are all great suggestions, thank you! I've made issues based on your suggestions and they will be addressed soon.
from automategitreposetup.
@funbeedev - I'm happy to hold on the unit testing, in addition I can pick up any other little bits you need help with (either coding or reviewing).
from automategitreposetup.
@Stedders Thanks, your help is appreciated!
See PR #16. This removes use of globals. Could you review the changes?
from automategitreposetup.
@Stedders I've made your suggested changes now and it's merged into the repo. For the relevant changes see PR #16 and #17.
Do you want to pull these changes and update the test in your PR?
Thanks!
from automategitreposetup.
Related Issues (15)
- Refactor to avoid use of global variables
- Pass the config file as a variable into the program
- Rename program file from automate-git.py -> automate_git.py
- Update docs to match current repo updates
- Upload original tutorial code to repo
- Update workflow to execute on any branch, not just main HOT 1
- Update with instructions on how to run test
- Rename and edit the Test workflow to match format of existing workflows
- Add .gitignore to repo
- Update links in contributing.md
- Add issue template Current Behaviour -> Changes Requested
- Port application to Windows
- Add option to create more than one repo at once
- Add exception handling
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from automategitreposetup.