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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated API: unfocusSearchFieldContainer, doUnfocusSearchFieldContainer() #10625

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Remove deprecated API: unfocusSearchFieldContainer, doUnfocusSearchFieldContainer() #10625

merged 1 commit into from
Jan 13, 2022

Conversation

SarveshMishra
Copy link
Contributor

@SarveshMishra SarveshMishra commented Jan 12, 2022 •
edited by vince-fugnitto
Loading

What it does

Fixes: #10622
Remove deprecated API: unfocusSearchFieldContainer, doUnfocusSearchFieldContainer()

How to test

confirm that theia builds and that example application it starts.

Review checklist

Reminder for reviewers

@JonasHelming JonasHelming requested a review from tsmaeder January 12, 2022 16:46
tsmaeder
tsmaeder previously requested changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@tsmaeder tsmaeder left a comment •
edited
Loading

Choose a reason for hiding this comment

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

When we remove API, we should add an entry to the "breaking changes" section of the changelog document.

@marcdumais-work
Copy link
Contributor

@SarveshMishra Please feel free to ping me on this issue here, for any further help needed. I think you just need to complete the CHANGELOG entry, amend your commit and force-push to your branch.

@SarveshMishra
Copy link
Contributor Author

@SarveshMishra Please feel free to ping me on this issue here, for any further help needed. I think you just need to complete the CHANGELOG entry, amend your commit and force-push to your branch.

@marcdumais-work I created a new entry in the CHANGELOG.md file v1.22.0 as you suggested please look into it.

@JonasHelming JonasHelming linked an issue Jan 12, 2022 that may be closed by this pull request
Remove deprecated API: unfocusSearchFieldContainer, doUnfocusSearchFieldContainer() #10622
Closed
@JonasHelming JonasHelming requested a review from tsmaeder January 12, 2022 17:35
@JonasHelming
Copy link
Contributor

@SarveshMishra Please feel free to ping me on this issue here, for any further help needed. I think you just need to complete the CHANGELOG entry, amend your commit and force-push to your branch.

@marcdumais-work I created a new entry in the CHANGELOG.md file v1.22.0 as you suggested please look into it.

Did you stage/commit the changelog.md? It does not show up in the PR

@SarveshMishra
Copy link
Contributor Author

@SarveshMishra Please feel free to ping me on this issue here, for any further help needed. I think you just need to complete the CHANGELOG entry, amend your commit and force-push to your branch.

@marcdumais-work I created a new entry in the CHANGELOG.md file v1.22.0 as you suggested please look into it.

Did you stage/commit the changelog.md? It does not show up in the PR

I amend the commit then forced push. Should I do another commit for this?

@SarveshMishra SarveshMishra force-pushed the GH-10622 branch 2 times, most recently from 81ed00c to cf02075 Compare January 12, 2022 18:06
@SarveshMishra
Copy link
Contributor Author

@SarveshMishra Please feel free to ping me on this issue here, for any further help needed. I think you just need to complete the CHANGELOG entry, amend your commit and force-push to your branch.

@marcdumais-work I created a new entry in the CHANGELOG.md file v1.22.0 as you suggested please look into it.

Did you stage/commit the changelog.md? It does not show up in the PR

I amend the commit then forced push. Should I do another commit for this?

After changing done in changelog.md file now it is showing conflict in files. Do I have to edit something on my side?

@marcdumais-work
Copy link
Contributor

After changing done in changelog.md file now it is showing conflict in files. Do I have to edit something on my side?

Probably someone has modified CHANGELOG.md and merged, since your branched-off for your PR.

You need to rebase your branch on the latest upstream master, and force-push again. e.g.:

$ git fetch origin
$ git rebase origin/master

@marcdumais-work
Copy link
Contributor

Thanks @SarveshMishra - looks good.

@tsmaeder can you have another look?

@vince-fugnitto vince-fugnitto dismissed tsmaeder’s stale review January 13, 2022 14:28

The 'breaking changes' changelog entry was correctly added.

vince-fugnitto
vince-fugnitto approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍 Thank you for your first contribution!

@marcdumais-work
Copy link
Contributor

We have a couple of approvals - let's merge this PR. Thanks @SarveshMishra for this first contribution to Eclipse Theia!

@marcdumais-work marcdumais-work merged commit 4952468 into eclipse-theia:master Jan 13, 2022
@SarveshMishra
Copy link
Contributor Author

We have a couple of approvals - let's merge this PR. Thanks @SarveshMishra for this first contribution to Eclipse Theia!

@marcdumais-work @JonasHelming @tsmaeder @vince-fugnitto Thanks for helping me with this first contribution.
So do I have to delete this branch and then create a fresh new branch for any next issue.., right??

@marcdumais-work
Copy link
Contributor

Thanks for helping me with this first contribution

You're welcome - thanks for participating to the hackathon.

So do I have to delete this branch and then create a fresh new branch for any next issue.

It's up-to you, but probably yes: the source branch is no longer needed (by us) after merge.

@SarveshMishra
Copy link
Contributor Author

Thanks for helping me with this first contribution

You're welcome - thanks for participating to the hackathon.

So do I have to delete this branch and then create a fresh new branch for any next issue.

It's up-to you, but probably yes: the source branch is no longer needed (by us) after merge.

Ok, now I will try to solve level 1 or 2 issues and try to create PR for them. Thanks

@SarveshMishra SarveshMishra deleted the GH-10622 branch January 13, 2022 16:24
@vince-fugnitto vince-fugnitto added the quality issues related to code and application quality label Jan 27, 2022
@vince-fugnitto vince-fugnitto added this to the 1.22.0 milestone Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JonasHelming JonasHelming JonasHelming approved these changes

@vince-fugnitto vince-fugnitto vince-fugnitto approved these changes

@tsmaeder tsmaeder Awaiting requested review from tsmaeder

Assignees
No one assigned
Labels
quality issues related to code and application quality
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

Remove deprecated API: unfocusSearchFieldContainer, doUnfocusSearchFieldContainer()
5 participants
@SarveshMishra @marcdumais-work @JonasHelming @tsmaeder @vince-fugnitto