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

User Management backend - Base - Project restructure #36

Merged
merged 5 commits into from
May 3, 2020

Conversation

DoRTaL94
Copy link
Collaborator

@DoRTaL94 DoRTaL94 commented Apr 25, 2020

fixes #27.

  • Renamed test folder to tests.
  • Renamed flask_init file to app and updated its name inside setup.sh.
  • Moved static and templates folders to edison folder.
  • Changed network definition to private and removed forwarded ports.
    You can read about the reason behind this here.

@ahinoamta @RoniRush

@DoRTaL94 DoRTaL94 changed the title Project restructure User Management backend - Base - Project restructure Apr 25, 2020
@DoRTaL94 DoRTaL94 requested a review from ifireball April 25, 2020 16:52
- Renamed test folder to tests.
- Renamed flask_init file to app and updated its name inside setup.sh.
- Moved static and templates folders to edison folder.
- Changed network definition to private.
- Added a dhcp bug fix.
Added init file to edison folder to make it a module.
config.vm.box = "ubuntu/bionic64"
config.vm.provision :shell, path: "setup.sh"
config.vm.network :forwarded_port, guest: FLASK_PORT, host: FLASK_PORT
config.vm.network :forwarded_port, guest: POSTGRESQL_PORT, host: POSTGRESQL_PORT
config.vm.network "private_network", type: "dhcp"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you forgot to mention this change in your PR description

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, they way you describe it there, tells people what you did - which should generally be obvious from the code, but not why you did it which is the one thing that code can never tell you on its own...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay I didn't think about it, thank you. I'll rewrite it.

Copy link
Collaborator Author

@DoRTaL94 DoRTaL94 Apr 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ifireball I referenced your comment in our discussion in the old PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll personally I prefer to write a short summary of what I want people to know, instead of sending then to look for it in a discussion thread, but that is not a reason to block a PR.

edison/app.py Outdated
from flask import render_template


# API description in swagger - https://app.swaggerhub.com/apis/DoRTaL94/UserManagment/1.0.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this comment relevant to this PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it would probably be relevent when we will actually add the app routes.
I'll remove it.

@DoRTaL94 DoRTaL94 requested a review from ifireball April 26, 2020 08:21
Changed the linked issue to a more relevant one
@lmilbaum lmilbaum merged commit 932511b into redhat-beyond:master May 3, 2020
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

Successfully merging this pull request may close these issues.

User Management backend - Base
5 participants