quran / quran.com-frontend

quran.com frontend
https://quran.com
MIT License
994 stars 360 forks source link

rewrite should not exist in its current form #200

Closed ahmedre closed 8 years ago

ahmedre commented 8 years ago

salam 3alaikum, the rewrite branch should not exist for a few reasons:

  1. it starts with a clean slate (i.e. does not preserve repository history)
  2. it contains lots of stuff that "isn't ours" (copy/paste code, etc), or isn't needed
  3. it is not stable (doesn't build cleanly without modifications)
  4. it is very difficult for anyone to actually review it
  5. it is very difficult to revert back or compare to older behaviors once rewrite is force-pushed to master

proposal:

  1. fix master (see #199) - tag it as stable-1.0 since that's what we've been running for a few months now in production.
  2. start porting relevant changes in "bite sized, well reviewed chunks" to either master or to a branch that is based off of master.

to be clear, the primary reason i am against rewrite is that it's not based off of master, and would require a force push to become master in the future. i support incremental rewrites rather than one time rewrites, but if it's needed, then it should be done without destroying history (and without losing automation tools built into git, like bisect, to figure out problems, etc).

i also understand that some rewrites are framework rewrites that really require throwing lots of old code away - even in this case, we should still do our work off of master. if you think about it, this sort of enables better code hygiene as well (i.e. separating out framework-specific code from non-framework specific code, for example).

am open to thoughts about this, but i strongly believe a clean slate in terms of git is definitely not the way to go.

finally, one last point - with web, there are new "hot frameworks" every day - if we rewrite the site every time there is a new framework, we will not be able to make much (if any) progress on it. instead, we should focus on fixing up stability issues, and deciding on a set of features to implement before Ramadan, and file tickets so people can pitch in.

walsalam 3alaikum.

mmahalwy commented 8 years ago

@ahmedre I agree with you. Let's close the rewrite branch and I will get the master to stable. It's unstable right now cause I upgraded too early before the core libs were stable (when babel 6 first came out, not many people supported it but now everyone does).

As for the framework rewrite, Redux is definitely the way to go and Fluxible is not. What is awesome about the Reactjs world is everything is a component and is apart from others, if you wish them to be. So we can have both frameworks running side by side at first and some pages using the new while others using the old until I can migrate them one by one. The landing page being obviously super easy and a good candidate as the first.

Let me come up with a plan of porting things over and start creating PRs for each section/block of things. How does that sound?

ahmedre commented 8 years ago

@mmahalwy i already brought master to stable in #199, you just need to accept it :) this is equivalent to the same build our servers are running today.

sure, that sounds like a great plan, jazakAllah khairan!

sharabash commented 8 years ago

forgive me for being ignorant, does that mean fix_master should build?

On Mon, Feb 29, 2016 at 11:55 PM Ahmed El-Helw notifications@github.com wrote:

@mmahalwy https://github.com/mmahalwy i already brought master to stable in #199 https://github.com/quran/quran.com-frontend/pull/199, you just need to accept it :) this is equivalent to the same build our servers are running today.

sure, that sounds like a great plan, jazakAllah khairan!

— Reply to this email directly or view it on GitHub https://github.com/quran/quran.com-frontend/issues/200#issuecomment-190414743 .

ahmedre commented 8 years ago

yes, master + #199 builds.

sharabash commented 8 years ago

what do you mean by master + #199? i.e., the app should build if i've merged in fix_master into master?

On Tue, Mar 1, 2016 at 11:55 AM Ahmed El-Helw notifications@github.com wrote:

yes, master + #199 https://github.com/quran/quran.com-frontend/pull/199 builds.

— Reply to this email directly or view it on GitHub https://github.com/quran/quran.com-frontend/issues/200#issuecomment-190640371 .

ahmedre commented 8 years ago

yep

azizur commented 8 years ago

As-salamu alaykum everyone,

Just before we start a re-write could I suggest we come up with series of feature/user stories so that we can all drive them using BDD and TDD.

What I am looking for is:

That way we can

Just my 2 cents!

ATouhou commented 8 years ago

Azizur even though I'm just following this project on the side line I 100% agree that it would be more effective and structured that way. Using GitHub issues to log what to do is not the best way 2 go

sharabash commented 8 years ago

we've got pivotal tracker, is that deficient? and if so what are the limitations that have to be addressed?

On Wed, Mar 2, 2016, 12:58 PM ATouhou notifications@github.com wrote:

Azizur even though I'm just following this project on the side line I 100% agree that it would be more effective and structured that way. Using GitHub issues to log what to do is not the best way 2 go

— Reply to this email directly or view it on GitHub https://github.com/quran/quran.com-frontend/issues/200#issuecomment-191188046 .

mmahalwy commented 8 years ago

@sharabash pivotal is awesome but setting people up and learning it is a pain. I am good for Jira or Trello

azizur commented 8 years ago

Another alternative is waffle.io check out the waffle.io borad itself.

azizur commented 8 years ago

@sharabash until this conversation, it was not even clear there was a pivotal tracker setup for this project.

On an open source project, this kind of information should be open in the public to encourage people to contribute. It could be from suggesting a new feature to I want a challenge myself and implement a feature or fix an issue that is annoying me. Without all those details in public space it hard to know what the team is working on and where a new contributor can help.

ahmedre commented 8 years ago

yep, i think we stopped using pivotal a long time ago, and no one besides core members knew about it unfortunately.

i started a public trello board here - nothing on it at the moment, but insha'Allah @mmahalwy can start filling in some stuff. once we get more stuff on it, insha'Allah we can add it to the README so others can find it.

mmahalwy commented 8 years ago

I think this has been met with closing the rewrite PR. closing this.