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

EC2Architect fixed warning when domain is in use #618

Merged
merged 38 commits into from
Dec 9, 2021

Conversation

Yash621
Copy link
Contributor

@Yash621 Yash621 commented Dec 7, 2021

This PR is in respect to issue #614 and continuation of PR #617
@JackUrb please review it , I have done all the changes that you suggested .

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 7, 2021
Copy link
Contributor

@JackUrb JackUrb left a comment

Choose a reason for hiding this comment

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

You're still making a lot of unrelated formatting changes in ec2_helpers.py - I don't think these are related to the given PR.

@Yash621
Copy link
Contributor Author

Yash621 commented Dec 7, 2021

@JackUrb actually i am not ,I am still figuring out why that file is being formatted ,can you please suggest how to fix this formatting issue ,I have ran black also on both the ec2_helpers.py and ec2_architect.py file but still getting formatting issue on the PR.

@JackUrb
Copy link
Contributor

JackUrb commented Dec 8, 2021

Well, for ec2_helpers.py at least you can just discard your changes on the file, as all of the changes there are formatting. When you run black, what is the output in your console? Perhaps you're not committing the black output changes.

@Yash621
Copy link
Contributor Author

Yash621 commented Dec 8, 2021

@JackUrb when i run black it shows
All done!
1 file left unchanged.
due to which i am not understanding that why even after formatting with black why it is not accepting code style ?

@JackUrb
Copy link
Contributor

JackUrb commented Dec 8, 2021

@JackUrb when i run black it shows All done! 1 file left unchanged. due to which i am not understanding that why even after formatting with black why it is not accepting code style ?

What is the full command you are running? Ideally you should pip install pre-commit so that black automatically runs on files in your checkout when you commit.

@Yash621
Copy link
Contributor Author

Yash621 commented Dec 9, 2021

@JackUrb I have fixed the formatting issues ,now their are no conflicts ,are there any additional changes that i need to do ?

Copy link
Contributor

@JackUrb JackUrb left a comment

Choose a reason for hiding this comment

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

Thanks for going through the manual effort here @Yash621. I'd really like to help figure out why the automated tooling isn't working for you.

At the very least, running pre-commit run --all-files should have been able to automatically run on your commit and made the changes, perhaps that's something to try next time?

In any case, thanks again for the contribution and for sticking this one out!

@JackUrb JackUrb merged commit fb83b3b into facebookresearch:main Dec 9, 2021
@Yash621
Copy link
Contributor Author

Yash621 commented Dec 9, 2021

@JackUrb ,surely i will try that next time !
no problem
always happy to contribute :)

@JackUrb JackUrb mentioned this pull request Dec 13, 2021
@pringshia pringshia linked an issue Jan 4, 2022 that may be closed by this pull request
@Yash621

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EC2Architect fix warning when domain is in use
3 participants