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

fix: icons are no longer displayed after css-loader major upgrade #1729

Merged
merged 1 commit into from
Sep 25, 2021

Conversation

ahochsteger
Copy link
Collaborator

This reverts commit 7063434.

Fixes #1728

@coveralls
Copy link

coveralls commented Sep 24, 2021

Pull Request Test Coverage Report for Build 1270762142

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 29.602%

Totals Coverage Status
Change from base Build 1268797050: 0.0%
Covered Lines: 3671
Relevant Lines: 13045

💛 - Coveralls

@ahochsteger ahochsteger changed the title Revert "chore(deps-dev): bump css-loader from 5.2.6 to 6.3.0 (#1723)" fix: icons are no longer displayed after css-loader major upgrade Sep 24, 2021
@ahochsteger ahochsteger enabled auto-merge (squash) September 24, 2021 13:23
@firstof9
Copy link

You also need to fix your commit message.

@ahochsteger
Copy link
Collaborator Author

You also need to fix your commit message.

@firstof9, I've seen the complaint about "Semantic PR title / commit message" and then already changed the title of the PR to comply to conventional commits.
Shouldn't this be enough?

@firstof9
Copy link

Nope your commit message also needs to follow suit.

Revert "chore(deps-dev): bump css-loader from 5.2.6 to 6.3.0 (#1723)"
This reverts commit 7063434.
@ahochsteger ahochsteger force-pushed the fix-css-loader-icon-problem branch from 5c8799b to 5ef6aa8 Compare September 24, 2021 17:29
@ahochsteger
Copy link
Collaborator Author

Nope your commit message also needs to follow suit.

@firstof9, done.

@firstof9
Copy link

Cheers!

@blhoward2
Copy link
Collaborator

blhoward2 commented Sep 25, 2021

@ahochsteger you can also try @zwave-js-bot rename commit next time. It only matters for single commit PRs. I think anyone can run that in their PR but may be wrong.

@ahochsteger ahochsteger merged commit b38caa2 into master Sep 25, 2021
Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

Ok, will try to fix the breaking changes once I come back. Thanks!

@ahochsteger ahochsteger deleted the fix-css-loader-icon-problem branch September 25, 2021 06:01
@ahochsteger
Copy link
Collaborator Author

@blhoward2, thanks, I was not aware, that for single-commit PRs the real commit message must be changed. I thought the PR title can be used as well for the merge commit.
I've done so with git commit --amend and git push --force (see this event) but I am unsure if this is really the way it should be done?
Usually I prevent force-pushing to Github ...

@blhoward2
Copy link
Collaborator

I'm not sure there is a right way. I've seen people force push. I've seen others push empty commits. I'm honestly not the expert because rarely do I do anything in one commit. 😬 We frequently use the bot to fix it for submitters.

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.

[bug] The icons are no longer displayed
5 participants