EddieHubCommunity / support

Community Help & Support and AEA (Ask Everyone Anything)
https://eddiehubcommunity.github.io/support/
MIT License
342 stars 159 forks source link

Please, review my open source project #698

Closed Ashutosh00710 closed 3 years ago

Ashutosh00710 commented 3 years ago

Items we can review on a live stream:

This project is related to GitHub readme(s). A dynamically generated activity graph to show your GitHub activities of last 31 days.

Work in progress and open for contribution for feature enhancement.

github-actions[bot] commented 3 years ago

It's great having you contribute to this project

Feel free to raise an Issue! Welcome to the community :nerd_face:

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 🤓 . Any questions let us know.

mikeysan commented 3 years ago

Hi @Ashutosh00710,

This looks really cool. I'll definitely play with it a bit more. The readme could do with a little update, though. The section about passing my username the url was immediately clear to me. I can see where I need to change, but do I need that entire line to try it out? I don't know just by looking at what's written in the readme.

[![Ashutosh's github activity graph](https://activity-graph.herokuapp.com/graph?username=Ashutosh00710)](https://github.com/ashutosh00710/github-readme-activity-graph)

I am guessing, for example, that I do not need this section.

[![Ashutosh's github activity graph]

Looking closer, it appears the only section I need is:

https://activity-graph.herokuapp.com/graph?username=Ashutosh00710

I would change the readme to reflect that.

Finally, looking at the graph itself. You don't have to, but you could add a footer at the bottom of the graph that points back to your github repo. This means, for example, If I use the link to display the graph somewhere outside GitHub (e.g. my website) others can click on the footer and it will take them to your repo.

That's my 2p. I hope it helps.

ps. I can make the README file changes and raise a PR if you like. pps. I love the Dracula theme :)

Ashutosh00710 commented 3 years ago

Hello! @mikeysan .Thank you so much for reviewing the project 🙂, it means a lot. Feel free to contribute to this project. You can raise an issue and then make your pull request.

We would love to have you as one of our contributor ❤.

About the footer idea I think it's a great idea. You can raise issue for this also to get another green square for the day 🟩.

We will definitely implement this feature ASAP.

mikeysan commented 3 years ago

I'll be honoured to help. Thanks.

I have raised an issue for the footer link.

I will raise another issue and work on the Readme shortly (after making dinner) :)

Vyvy-vi commented 3 years ago

Your github-profile looks great :+1:

I plan on checking out the project later... It looks interesting 🙃

Ashutosh00710 commented 3 years ago

Thank you @Vyvy-vi for your appreciation, feedback and attention. 🙂

This community is really very active, responsive and supporting. I am glad to be a part of this community. Thank you @eddiejaoude for all this 🙂

eddiejaoude commented 3 years ago

Thank you for sharing 👍 . I will review in a live stream or video soon 🤓

Vyvy-vi commented 3 years ago

Some Feedback/Suggestions:

Ashutosh00710 commented 3 years ago

Hey, @Vyvy-vi I am highly impressed by your efforts in having a closer look at the project. Thanks for your Feedback/Suggestions.

I see a development branch there... It seems to be merged into the main, but right now is 33 commits behind main.

It was 33 commits behind because I was experimenting with some major changes in the code which may break the tool.

There's a line in CONTRIBUTING.md file that gives instructions on how to sync your fork. This is a great open source practice, but I think it needs a slight update. The current line states git pull upstream master, but I think there is no master branch of the repo.

Yes, great point, thanks for driving my attention towards that.

The CONTRIBUTING.md file doesn't state npm install before npm start (this might throw off new contributers, that haven't used npm before)

Another, good point.

I see an easy label. Now, that is completely fine, but personally using easy as a comparison on issues and PRs might be slightly demeaning/demotivating, and maybe what's easy for you might not be easy for someone else.. Perhaps look into slightly different language like beginner friendly or first timers.

Yes, I totally agree, I'll rename the tag soon.

mikeysan commented 3 years ago

Excellent feedback @Vyvy-vi

@Ashutosh00710 I just commented on a closed issue on your repo. I'm glad to see someone else got the green square for the footer :)

eddiejaoude commented 3 years ago

Great project, reviewed on a live stream, some of the EddieHub feedback..

https://youtu.be/djpH43hsOJI

Ashutosh00710 commented 3 years ago

Thank You @eddiejaoude and EddieHub for reviewing my project on the live stream.

I've learned all the valuable Open Source things from you and your YouTube Page.

Great contributing.md file 👍

Loved this ❤

Why have a __test__ folder, is this naming special, looks like python standard?

It contains tests related to project.

Suggest taking a tag/release

Sure I do.

Special thanks to @eddiejaoude @nhcarrigan @Vyvy-vi @mikeysan

For their valuable suggestion, feedback and support. They are proof of Collaboration First, Code Second. ❤

Grateful to each one of you.

naomi-lgbt commented 3 years ago

It contains tests related to project.

Right but why is it named with underscores like that? Instead of just test or tests?

Ashutosh00710 commented 3 years ago

Right but why is it named with underscores like that? Instead of just test or tests?

I was new to writing tests and watched __test__ naming in a video that's why I didn't know the standards. I'll rename it surely.

Vyvy-vi commented 3 years ago

Btw, we don't use __test__ for the test folder in python :person_shrugging: afaik