chaostoolkit-incubator / chaostoolkit-aws

Chaos Toolkit Extension for AWS
https://chaostoolkit.org/drivers/aws/
Apache License 2.0
109 stars 52 forks source link

remove legacy py2-py3.5 compatability syntax #99

Closed graingert closed 3 years ago

ciaransweet commented 3 years ago

Hi @graingert - Sorry that this PR has been lost in the void

I've just joined the project and I'm also looking to work on the AWS extension a fair bit. I inadvertently just made a change which is a fair bit of what you're also doing here.

I feel very cheeky asking you to revive a ~year old PR, so if it's alright with you, I'll close this and raise an Issue to cover off the changes I've not yet done.

Please let me know if you'd rather get this PR up-to-date with the current state of the repo and do this yourself, but I am more than happy for you to not 😛

graingert commented 3 years ago

Hi @graingert - Sorry that this PR has been lost in the void

I've just joined the project and I'm also looking to work on the AWS extension a fair bit. I inadvertently just made a change which is a fair bit of what you're also doing here.

I feel very cheeky asking you to revive a ~year old PR, so if it's alright with you, I'll close this and raise an Issue to cover off the changes I've not yet done.

Please let me know if you'd rather get this PR up-to-date with the current state of the repo and do this yourself, but I am more than happy for you to not stuck_out_tongue

done

graingert commented 3 years ago

I highly recommend you install https://pre-commit.ci/

ciaransweet commented 3 years ago

@graingert great thanks!

I agree, something like pre-commit will be very useful. We're working on corralling all the CTK repositories to be more aligned so this will definitely be something I look at introducing to make all our lives easier

For now though, whilst we agree as a team what we use, could I ask you take out the pre-commit setup you've got here in the PR, other than that, once the builds pass, this should be 💯 to go in.

If you have good arguments for pre-commit over other things (another cheeky ask coming) - Could you raise an issue https://github.com/chaostoolkit/chaostoolkit here with just a very brief TL;DR about why you'd put it into a project?

Saves us not losing track of it! Thanks!

ciaransweet commented 3 years ago

Also, being a pain too. We've got the DCO checks that should pass.

https://github.com/chaostoolkit-incubator/chaostoolkit-aws/pull/99/checks?check_run_id=3299634384

Probably the easiest way is to squash all your commits into one and sign that off, rather than having to figure out which ones are and aren't signed off.

ciaransweet commented 3 years ago

(Sorry, last one) - Could you also add an entry to CHANGELOG.md under ### Changed

ciaransweet commented 3 years ago

Closing due to inactivity - thanks for raising the original PR!

I've moved the changes into #119