Skip to content

refactor(material/snack-bar): refactor variable type to const in snack-bar component unit tests #22774

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

Merged
merged 1 commit into from
May 29, 2021

Conversation

nitinmalave
Copy link
Contributor

Opening this PR as it is required to:

  • Refactor variables in snack-bar component unit tests to have constant type instead of let as this variables are not getting re-initialize during the test life-time.
  • All tests within a spec should be consistent with all other tests in file.

@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 24, 2021
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

The change looks good, but the branch needs to be rebased.

Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

Yeah, changes in 425edb5 look good. The PR shows more commits though, most likely because it isn't rebased on top of master. Can you please fix that?

@nitinmalave
Copy link
Contributor Author

nitinmalave commented May 26, 2021

Hello @devversion, i am pretty new to this project and trying to contribute for first time in angular materials. I am not fully aware about rebasing branches in git. Can you please help me with the process to follow in order to get this commit merged in master?

Previously i did "git rebase master" on my local branch and pushed the changes on remote branch. What can be done from here? Any help would be appreciated. Thank you!

@nitinmalave nitinmalave force-pushed the refactor-snackbar-unit-tests branch 2 times, most recently from d715199 to 09ad4e7 Compare May 28, 2021 16:05
…k-bar component unit tests

style(material/snack-bar): fix tslint trailing whitespace issue in ci build
@nitinmalave nitinmalave force-pushed the refactor-snackbar-unit-tests branch from 9141a22 to b4b8104 Compare May 28, 2021 16:51
@nitinmalave
Copy link
Contributor Author

nitinmalave commented May 28, 2021

Hello @devversion @crisbeto, I have rebased the branch on top of master and pushed all changes. All checks have passed as of now. Please take a look of PR. Thanks!

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

LGTM

@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker merge safe target: patch This PR is targeted for the next patch release labels May 29, 2021
@mmalerba mmalerba merged commit 2a401e8 into angular:master May 29, 2021
mmalerba pushed a commit that referenced this pull request May 29, 2021
…k-bar component unit tests (#22774)

style(material/snack-bar): fix tslint trailing whitespace issue in ci build

(cherry picked from commit 2a401e8)
@nitinmalave nitinmalave deleted the refactor-snackbar-unit-tests branch May 30, 2021 05:54
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants