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

refactor: remove file-loader and use asset modules instead #1855

Merged
merged 3 commits into from
Oct 13, 2021
Merged

refactor: remove file-loader and use asset modules instead #1855

merged 3 commits into from
Oct 13, 2021

Conversation

damianstasik
Copy link
Contributor

@damianstasik damianstasik commented Oct 12, 2021 •
edited
Loading

New Pull Request Checklist

Issue Description

A small refactor that replaces file-loader with the built-in asset module available in webpack 5.

Related issue: #1855

Approach

Removed old file-loader and replaced it with asset module.

TODOs before merging

  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Oct 12, 2021 •
edited
Loading

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@mtrezza
Copy link
Member

mtrezza commented Oct 12, 2021 •
edited
Loading

Nice! please let me know when this is ready for review, I think you cannot request a review in the UI yet. And could you please remove the TODOs in the template that do not apply to the PR? So we know whether this is good to go.

@damianstasik
Copy link
Contributor Author

@mtrezza thanks! It is ready for review, just have one question about the PR template: is there anything missing from it or the bot is just looking for those removed checkboxes?

@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021 •
edited
Loading

The bot is complaining about not using the template when delete one or more headings that are in the template. I suggest you keep all headings as they are and just fill out "n/a" is a heading doesn't apply. We will simplify the templates soon.

@damianstasik
Copy link
Contributor Author

@mtrezza somehow the first heading got lost, I added it back, but the bot does not like removed checkboxes at the top so I added back the second checkbox with a link to the PR and now it is fine.

@mtrezza
Copy link
Member

mtrezza commented Oct 13, 2021 •
edited
Loading

Well done. Obviously, filling out a template is somewhat tedious, but if 1 author spends 10 seconds more on filling it out, it saves each future reader minutes of time to grasp the issue. It also helps reviewers to faster assess the issue and comment on it, which helps to prevent issues from getting stale.

mtrezza
mtrezza approved these changes Oct 13, 2021
View reviewed changes
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good! You are the king of package slimmers!

@mtrezza mtrezza merged commit 49d0b5a into parse-community:alpha Oct 13, 2021
@parseplatformorg
Copy link
Contributor

🎉 This pull request has been released in version 3.3.0-alpha.5

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Oct 15, 2021
@mtrezza mtrezza mentioned this pull request Oct 21, 2021
Closed
5 tasks
@snyk-bot snyk-bot mentioned this pull request Oct 31, 2021
Merged
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 3.3.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Nov 1, 2021
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 3.3.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Jan 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtrezza mtrezza mtrezza approved these changes

Assignees
No one assigned
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@damianstasik @mtrezza @parseplatformorg @visualfanatic