Skip to content

Refactoring#204

Merged
arabnejad merged 26 commits intomasterfrom
refactoring
Jun 8, 2021
Merged

Refactoring#204
arabnejad merged 26 commits intomasterfrom
refactoring

Conversation

@arabnejad
Copy link
Copy Markdown
Collaborator

@arabnejad arabnejad commented May 26, 2021

As we discussed in #203, I did some changes on files and folders in FabSim3 and put all FabSim3 python code files in a separate subdirectory, i.e., `fabsim. The repository's root directory is only used for other stuffs such as installation scripts, py tests, and all manner of other things that aren't actually part of the source code.

@djgroen : Another thing is that, if the user want to use fab command instead of fabsim, it should be called inside FabSim3/fabsim or FabSim3/plugins, I already added fabfile.py file in the plugins folder, but if you like to have access fab command from root directory, we can move fabfile.pyto the repository's root directory

@djgroen
Copy link
Copy Markdown
Owner

djgroen commented May 26, 2021

Hi Hamid.

Yes, I think fabfile.py should be in FabSim3's root directory, as I think people are very used to running it from there (or from the plugin directories) :).

@djgroen djgroen assigned djgroen and arabnejad and unassigned djgroen May 26, 2021
@djgroen
Copy link
Copy Markdown
Owner

djgroen commented May 26, 2021

I think that aside from that minor issue, this code is ready to be merged in.

We still have 2-3 weeks to resolve issues if they arise after the merge, and I think we'll detect them rapidly as we're all using FabSim3 extensively at the moment :).

@arabnejad
Copy link
Copy Markdown
Collaborator Author

@djgroen
in the refactoring version, running fab can be called in any subdir inside FabSim3/fabsim or FabSim3/plugins
but if you like, we can easily move it to root directory

@arabnejad
Copy link
Copy Markdown
Collaborator Author

@djgroen

I moved the fabfile.py to the root directory and also fixed its path in /fabsim/bin/fabsim executable file

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented May 26, 2021

This pull request introduces 1 alert and fixes 1 when merging 07bca8c into 29e6b59 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 1 for Use of the return value of a procedure

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented May 28, 2021

This pull request introduces 1 alert and fixes 1 when merging c4ffaf5 into 29e6b59 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 1 for Use of the return value of a procedure

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Jun 2, 2021

This pull request introduces 1 alert and fixes 1 when merging 81185ea into 29e6b59 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 1 for Use of the return value of a procedure

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Jun 2, 2021

This pull request introduces 1 alert and fixes 1 when merging 44a63fe into 29e6b59 - view on LGTM.com

new alerts:

  • 1 for Use of the return value of a procedure

fixed alerts:

  • 1 for Use of the return value of a procedure

@lgtm-com
Copy link
Copy Markdown

lgtm-com Bot commented Jun 8, 2021

LGTM pull request analysis was skipped for bb4b07c by arabnejad. Analysis of future commits will happen as normal.

@arabnejad arabnejad merged commit b221217 into master Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants