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

Remove pwsh #1752

Merged
merged 30 commits into from
Apr 11, 2024
Merged

Remove pwsh #1752

merged 30 commits into from
Apr 11, 2024

Conversation

JimMadge
Copy link
Member

@JimMadge JimMadge commented Mar 1, 2024

✅ Checklist

  • You have given your pull request a meaningful title (e.g. Enable foobar integration rather than 515 foobar).
  • You are targeting the appropriate branch. If you're not certain which one this is, it should be develop.
  • Your branch is up-to-date with the target branch (it probably was when you started, but it may have changed since then).
  • You have marked this pull request as a draft and added '[WIP]' to the title if needed (if you're not yet ready to merge).

⤴️ Summary

Remove pwsh stuff, preparing to merge v5.0.0 work into develop branch

🌂 Related issues

🔬 Tests

This comment was marked as off-topic.

Copy link
Member

@jemrobinson jemrobinson left a comment

Choose a reason for hiding this comment

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

Can we hold off on this until 4.2.0 is finalised? I'd like to merge main into python-migration once that's done (so we get the updated docs etc.) and merging this PR will cause a lot of conflicts when we do that.

@JimMadge
Copy link
Member Author

JimMadge commented Mar 1, 2024

Hmm, I think we will have conflicts either way. Unless we close this and repeat the steps again later.

I don't think I'm very worried about conflicts here, they should mostly be "change vs remove" which is pretty straightforward. (I'm volunteering to manage the conflicts 😄)

@JimMadge
Copy link
Member Author

JimMadge commented Mar 5, 2024

@craddm what do you think?

@craddm
Copy link
Contributor

craddm commented Mar 5, 2024

Is leaving it in place until v4.2.0 going to cause any problems? Inclined to leave as is, so that develop can be brought back to parity with v4.2.0 beforehand. Also aren't there a couple of potential issues we may want to do as a 4.2.1 that still need the powershell codebase?

@jemrobinson
Copy link
Member

My view is that we should do things in this order:

  1. get 4.2.0 tagged from develop
  2. merge develop into latest
  3. merge develop into python-migration
  4. remove unnecessary things from python-migration and/or port any fixes that have been made in Powershell <- THIS PR
  5. merge python-migration into develop

@JimMadge
Copy link
Member Author

JimMadge commented Mar 5, 2024

Is leaving it in place until v4.2.0 going to cause any problems?

Nope.

so that develop can be brought back to parity with v4.2.0 beforehand.

I think you are both keen on this. I can't really see the benefit, especially as we are going to immediately merge python-migration into develop anyway. Not a hill I will die on though so we can leave this PR open.

Also aren't there a couple of potential issues we may want to do as a 4.2.1 that still need the powershell codebase?

Yes but those changes won't be made on develop. That branch can start from v4.2.0

  1. get 4.2.0 tagged from develop
  2. merge develop into latest
  3. merge develop into python-migration

I think there are a few things wrong with that (hill I will die on)
The process should be

  1. Create release branch from develop
  2. Apply any changes needed for release
  3. Merge release branch into latest
  4. Push tag to latest
  5. Merge latest into develop

I.e. gitflow

Otherwise,

  • The tag is on your develop branch, not the head of latest
  • You merge changes from develop not on your release branch into latest (which in our case means changes which haven't been tested!)

@jemrobinson
Copy link
Member

jemrobinson commented Mar 5, 2024

I agree with the release-ordering things that @JimMadge said :)

@JimMadge JimMadge marked this pull request as draft March 5, 2024 22:29
@jemrobinson
Copy link
Member

We need to confirm that #1771 is done before merging this.

@jemrobinson jemrobinson changed the title Remove pwsh [WIP] Remove pwsh Apr 8, 2024
Base automatically changed from python-migration to develop April 8, 2024 13:20
dependabot bot and others added 8 commits April 8, 2024 13:21
Bumps [cryptography](https://github.com/pyca/cryptography) from 41.0.7 to 42.0.4.
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@41.0.7...42.0.4)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
…yptography-42.0.4

Bump cryptography from 41.0.7 to 42.0.4
Co-authored-by: Jim Madge <[email protected]>
@JimMadge JimMadge marked this pull request as ready for review April 9, 2024 13:01
@JimMadge JimMadge changed the title [WIP] Remove pwsh Remove pwsh Apr 9, 2024
@JimMadge JimMadge requested a review from jemrobinson April 9, 2024 14:37
Copy link
Contributor

@craddm craddm left a comment

Choose a reason for hiding this comment

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

LGTM, would keep the devcontainer though.

@JimMadge
Copy link
Member Author

LGTM, would keep the devcontainer though.

💯 Didn't see the devcontainer files for Python (or, they got deleted in a merge and I wasn't paying attention 😄)

@JimMadge JimMadge requested a review from craddm April 11, 2024 14:17
@JimMadge JimMadge merged commit e8c737a into develop Apr 11, 2024
11 checks passed
@JimMadge JimMadge deleted the remove_pwsh branch April 11, 2024 14:26
@jemrobinson
Copy link
Member

Um, I think we missed the part where we needed to do #1771 (this is an issue not a PR) before merging this. Can we work on this next so it doesn't get missed?

@JimMadge
Copy link
Member Author

JimMadge commented Apr 11, 2024

Oops, I assumed that was the dev -> python_migration PR 🤦 . Sorry, yes let's prioritise that. The updated configuration is all still in the commit history.

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.

3 participants