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

Removed empty cell #3234

Merged
merged 6 commits into from Jan 13, 2022
Merged

Removed empty cell #3234

merged 6 commits into from Jan 13, 2022

Conversation

MaheshMoholkar
Copy link
Contributor

@MaheshMoholkar MaheshMoholkar commented Jan 12, 2022

Pull Request Description

Removed empty cell from docs/source/demos/text_input.ipynb "Fixes #3206"

@@ -7,6 +7,7 @@ Release Notes
* Fixed classification pipelines to only accept target data with the appropriate number of classes :pr:`3185`
* Added support for time series in ``DefaultAlgorithm`` :pr:`3177`
* Standardized names of featurization components :pr:`3192`
* Removed empty cell :pr:`3234`
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add some more color here? Something like, "Removed empty cell in text_input.ipynb :pr:3234" would be great!

@jeremyliweishih
Copy link
Contributor

Thanks for putting up this new PR @Mahesh1822, just had one comment and I can approve after. In the future you do not need to put up a new PR! You can just make changes to the branch you're on, push the branch up, and the change will be in your PR as well. Let me know if that makes sense and how else I can help. Since a commit has been merged into our main (base) branch, you will need to update this branch by merging in the latest changes from main as well before you can merge this PR.

@codecov
Copy link

codecov bot commented Jan 12, 2022

Codecov Report

Merging #3234 (745e191) into main (0bae399) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3234   +/-   ##
=====================================
  Coverage   99.8%   99.8%           
=====================================
  Files        326     326           
  Lines      31395   31395           
=====================================
  Hits       31304   31304           
  Misses        91      91           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0bae399...745e191. Read the comment docs.

@jeremyliweishih
Copy link
Contributor

Another handy thing to note is: if you add "Fixes #3206" in your PR description, it will automatically close that issue once this PR goes in. Heres some documentation from github on how that works: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue!

@MaheshMoholkar
Copy link
Contributor Author

Thanks I did the changes and updated my repo too. Please check if everything is right.

Copy link
Contributor

@jeremyliweishih jeremyliweishih left a comment

Choose a reason for hiding this comment

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

Thank you this is great! Should be good once tests pass!

@MaheshMoholkar
Copy link
Contributor Author

It's showing a conflict. Why is it?

@MaheshMoholkar
Copy link
Contributor Author

I edited the doc a bit. And it solved the conflict.

@jeremyliweishih
Copy link
Contributor

jeremyliweishih commented Jan 13, 2022

@Mahesh1822 in general a conflict arises if someone else merged a change into main that touches the files in this PR. I didn't see the exact conflict but I imagine it was because someone made a change in main and had a change in the release notes that coincided with yours. Thanks for fixing the conflict!

Heres the GitHub docs on it: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/addressing-merge-conflicts/resolving-a-merge-conflict-on-github but you can also make the changes locally and push it up. Feel free to merge once tests pass!

@chukarsten
Copy link
Collaborator

Thanks for contributing @Mahesh1822 !

@jeremyliweishih
Copy link
Contributor

@Mahesh1822 I'm going to merge before another conflict arises. Thank you!

@jeremyliweishih jeremyliweishih merged commit 95a9859 into alteryx:main Jan 13, 2022
@MaheshMoholkar
Copy link
Contributor Author

Here goes my first contribution : )

@chukarsten chukarsten mentioned this pull request Jan 18, 2022
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.

Blank Cell in Documentaion
3 participants