Skip to content

Conversation

@smeet07
Copy link
Contributor

@smeet07 smeet07 commented Aug 27, 2022

fixing repr issue

Please add a meaningful description for your change here


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Choose reviewer(s) and mention them in a comment (R: @username).
  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI.

@github-actions
Copy link
Contributor

Assigning reviewers. If you would like to opt out of this review, comment assign to next reviewer:

R: @yeandy for label python.

Available commands:

  • stop reviewer notifications - opt out of the automated review tooling
  • remind me after tests pass - tag the comment author after tests pass
  • waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

@yeandy
Copy link
Contributor

yeandy commented Aug 29, 2022

Can you please give more context on this change?

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2022

Reminder, please take a look at this pr: @yeandy

@smeet07
Copy link
Contributor Author

smeet07 commented Sep 8, 2022

Can you please give more context on this change?

Can you please give more context on this change?

Tried method overriding but didn't do it properly, also new to open source so I don't know what happens next

@yeandy
Copy link
Contributor

yeandy commented Sep 8, 2022

Thanks for the info, and contributing to Beam! In general, you're doing the right steps. You did take-issue on the original issue, and also noted the issue number in the title. I didn't realize at first; hence my question about context. I would be helpful to link the issue in the description of this PR 😄

The next steps would to address any PreCommit checks that are failing. You have a few
Screen Shot 2022-09-08 at 4 53 04 PM
You can see what is wrong if you click under Details.

We have a bot to automatically assign reviewers. (This time, it was me). You can also manually choose a reviewer if you know someone in particular you want to review.

However, @AnandInguva and I are actually working on a bunch of changes (#22995) in preparation to support Python 3.10. While working, I had identified the need to address the issue (#20982) that you originally took, and made a PR (AnandInguva#4) already. I guess we accidentally started working on this at the same time 😅

I'd say you're welcome to continue work on this PR (and also take inspiration from what I've done). Depending on how fast/slow our 3.10 work takes (I can update you via this PR), you may be able to make this fix into master before us? But I also don't want there to be redundant work being done.

@AnandInguva How do you think we should approach this?

@AnandInguva
Copy link
Contributor

Both of you, Thanks for working on this. This is a blocker for Python 3.10 and it would be better if we could do it sooner than later. Andy has started some work and I think is about to be done with it.

@smeet07
Copy link
Contributor Author

smeet07 commented Sep 10, 2022

Thank you for guiding @yeandy @AnandInguva , I guess I'll leave this issue to you both and try contributing to some other issue

@smeet07
Copy link
Contributor Author

smeet07 commented Sep 10, 2022

Also is there any way to see whether the checks are failing before creating a pr?

@yeandy
Copy link
Contributor

yeandy commented Sep 12, 2022

Yes. It's at the bottom of this page. You can click on Details on the right side to see more info for any particular test.
Screen Shot 2022-09-12 at 9 24 46 AM

@smeet07 smeet07 closed this Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants