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.

Sign up for GitHub

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

Jump to bottom

Enhancement: Docker Swarm stack name added. #26

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Enhancement: Docker Swarm stack name added. #26

wants to merge 1 commit into from

Conversation

adityaofficial10
Copy link

Fixes issue: odk-x/tool-suite-X#194

What changes have been made?

  • The docker swarm stack name can be input by user now.
  • The default value is 'syncldap'
  • The stack name will be saved in the settings file.

@adityaofficial10
Copy link
Author

@linl33
Kindly review this.

@wbrunette wbrunette requested review from r0ssing and linl33 March 9, 2021 19:21
@adityaofficial10
Copy link
Author

@linl33
Is this fine or Can I improve this further?

linl33
linl33 requested changes Mar 11, 2021
View reviewed changes
Copy link
Member

@linl33 linl33 left a comment

Choose a reason for hiding this comment

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

2 small issues but otherwise the script works.

I noticed that your commit was based off of the master branch. There are changes in the development branch that are useful for testing.

init-odkx-sync-endpoint.py
@@ -97,8 +105,7 @@ def run_interactive_config():
--non-interactive".format(email, domain))

print("Attempting to save updated https configuration")
write_to_env_file(env_file_location, domain, email)

write_to_env_file(env_file_location, domain, email, stack_name)
Copy link
Member

Choose a reason for hiding this comment

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

This should not be guarded behind enforce_https anymore.

init-odkx-sync-endpoint.py
@@ -19,7 +19,7 @@ def run_interactive_config():
env_file_location = os.path.join(os.path.dirname(__file__), "config", "https.env")

try:
domain, email = parse_env_file(env_file_location)
domain, email, stack_name = parse_env_file(env_file_location)
Copy link
Member

Choose a reason for hiding this comment

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

For a user coming from a previous version, stack_name would be None here. Please add a check.

@r0ssing
Copy link
Member

r0ssing commented Mar 11, 2021

This looks fine to me 👍

@r0ssing r0ssing closed this Mar 11, 2021
@r0ssing r0ssing reopened this Mar 11, 2021
@r0ssing
Copy link
Member

r0ssing commented Mar 11, 2021 •
edited
Loading

(oops - sorry I fumbled - didn't mean to close it.)

@wbrunette
Copy link
Member

@adityaofficial10 please take a look at the PR feedback and possibly make the requested changes to address the issues. You can push additional commits to the same branch and it will automatically add it to the PR.

@adityaofficial10
Copy link
Author

@wbrunette
Sorry for the delay, I reviewed the changes requested by @linl33 but unfortunately had a health problem.
I'll start working again tomorrow..

@wbrunette
Copy link
Member

@adityaofficial10 no worries. We are trying to provide more directions for GSoC/Outreachy since it may be someone's first time involved in open-source projects.

@adityaofficial10
Copy link
Author

@wbrunette
I appreciate it and I have also been benefited by the guidance provided by you and @linl33 .
Thanks..

@linl33
Copy link
Member

linl33 commented Mar 23, 2021

@adityaofficial10 Could you give us an update on this pull request? Let us know if you're having any difficulties or questions.

@adityaofficial10
Copy link
Author

@linl33
I'm just having some health problems.
Kindly persist with me.

@wbrunette wbrunette requested review from linl33 and r0ssing April 11, 2021 21:35
@linl33
Copy link
Member

linl33 commented Apr 12, 2021

@wbrunette I'm still waiting on changes requested in the previous review.

@wbrunette
Copy link
Member

@adityaofficial10 can you give us an update on your progress? Do you have any questions?

@wbrunette wbrunette self-requested a review April 28, 2021 20:07
@wbrunette
Copy link
Member

@adityaofficial10 can you give us an update on your progress? Do you have any questions?

@wbrunette wbrunette mentioned this pull request Mar 27, 2022
Open
@Ndacyayisenga-droid
Copy link

Ndacyayisenga-droid commented Feb 12, 2023 •
edited
Loading

@wbrunette @linl33
Can I finish up with this if its still open?
Regards,
Noah

@MariaAngella
Copy link

Hello team,

I'm an Outreachy applicant for the winter 2023 season and have been assigned issue: issue: https://github.com/odk-x/tool-suite-X/issues/194
Following up here, @wbrunette @r0ssing @linl33 any update on how to resolve these conflicts on the branch?

Regards,
Maria Angella

@wbrunette
Copy link
Member

@MariaAngella you should start fresh with your own branch and learn about Git.

@MariaAngella
Copy link

@wbrunette will work from my branch thanks.

Regards,
Maria Angella Aboyo

@MariaAngella
Copy link

@wbrunette , kindly advise if I'm on the right track my branch

Thank you and regards,
Maria Angella

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@linl33 linl33 Awaiting requested review from linl33

@r0ssing r0ssing Awaiting requested review from r0ssing

@wbrunette wbrunette Awaiting requested review from wbrunette

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@adityaofficial10 @r0ssing @wbrunette @linl33 @Ndacyayisenga-droid @MariaAngella