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

Misc: Remove unnecessary assets and imports #59

Closed
wants to merge 3 commits into from

Conversation

Brijeshthummar02
Copy link

issue #57

closses #58

Copy link

netlify bot commented Sep 28, 2024

Deploy Preview for parthmittal ready!

Name Link
🔨 Latest commit b6641d0
🔍 Latest deploy log https://app.netlify.com/sites/parthmittal/deploys/67014a1a32d224000878747e
😎 Deploy Preview https://deploy-preview-59--parthmittal.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

src/assets/index.js Outdated Show resolved Hide resolved
Copy link
Owner

@mittal-parth mittal-parth left a comment

Choose a reason for hiding this comment

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

  • Don't comment files that are required. That's breaking the build
  • Commit messages are written in the imperative tense. For instance, Fix loading issue instead of fixed loading issue
  • Prefix each commit with something that describes its category. Here it can be misc:
  • When you mention 'closes #something' in the PR description, GitHub closes that issue automatically once merged. So specify the issue in this case not the PR number.

@mittal-parth mittal-parth changed the title unnecessary assets and imports. Misc: Remove unnecessary assets and imports Sep 28, 2024
@Brijeshthummar02
Copy link
Author

@mittal-parth req update.

@mittal-parth
Copy link
Owner

@Brijeshthummar02 As I said earlier also, you have to remove unnecessary imports also.

For instance, here

import robot from "./robot.png";
import send from "./Send.svg";
import shield from "./Shield.svg";

....

export {
  robot,
  send,
  shield,
  ...
}

These imports are not used beyond this file meaning they are never used. How did I find that? Go through each asset that is imported in this file. Do a global search for that (like robot here) in your code editor (Ctrl + Shift + F for VSCode on Linux/Windows). If that doesnt occur beyond this file, then its not being used.

In contrast some file like ethforall is being imported and used in src/constants/index also which means it should not be removed.

@mittal-parth
Copy link
Owner

Hi @Brijeshthummar02, still working on it?

@Brijeshthummar02
Copy link
Author

By mistake i deleted the repo from system so i can't edit the current pr need to create new if its okayy.

@mittal-parth
Copy link
Owner

@Brijeshthummar02 you don’t have to make a new PR. Your fork still exists on GitHub. Just pull that and you are good to go

@Brijeshthummar02
Copy link
Author

@mittal-parth now is it ok?

@mittal-parth
Copy link
Owner

Have you checked if these are all imports to be removed? I just gave an example.

Please read the entire comment again and lemme know.

@mittal-parth
Copy link
Owner

Hi @Brijeshthummar02! Let’s close this soon?
thanks!

@Brijeshthummar02
Copy link
Author

i'm working on it.

@Brijeshthummar02
Copy link
Author

give me complete list of which one to remove

@mittal-parth
Copy link
Owner

There is nothing left for you to do if I do that myself. The point of the issue was to explore and get a hang of the project. I have given enough steps for you to be able to do it comfortably.

If you won’t be able to pick it up, lemme know.

@Brijeshthummar02 Brijeshthummar02 closed this by deleting the head repository Oct 9, 2024
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.

2 participants