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

Pull forward old dev branch changes #202

Closed
wants to merge 45 commits into from

Conversation

HebaruSan
Copy link
Contributor

Background

The dev branch, now moved to dev-old, had many commits that were developed and reviewed but not deployed to production. #133 and #163 were developing more changes on top of this branch, so we would like to keep these changes alive and not lose the work that they represent.

Changes

This pull request attempts to integrate the old dev branch's commits into the new alpha/beta/master server paradigm, see https://github.com/KSP-SpaceDock/SpaceDock/wiki/Migration-notes-and-tasks.

  1. Rebased a copy of the dev-old branch onto alpha. "theirs" means to resolve conflicts in favor of the contents of dev-old ("ours" and "theirs" are swapped for rebase).
    git checkout -b from-dev-old-theirs dev-old
    git rebase alpha -X renormalize -X ignore-space-change -X theirs
  2. Changed all CRLFs in each commit to LF, since the diffs were cluttered with ^Ms and whole-file conflicts.
    git filter-branch --tree-filter 'git ls-files -z | xargs -0 dos2unix' -- alpha..HEAD
  3. Squashed a few "fix" commits to simplify the history. Could probably use more of this.
    git rebase -i alpha
  4. Added a .gitattributes file to prevent CRLFs from getting into the repo in the future.

This should be merge-able into alpha without conflicts.
FYI to @RockyTV since this came up on the IRC channel.

Process

Note that I can't run this locally and I do not know how stable the old branch was, so it's possible that it breaks things. Close reading of the diffs may be advisable.

If this PR is merged, I will log into the alpha server and pull these changes, then restart the server manually to allow us to test them. If there are problems, we will investigate them and create further pull requests to fix them.

If it looks good, a future pull request will migrate the changes on the alpha branch to the beta branch, and they will be tested on the beta server as well.

Later, another PR would move them to the master branch, whereupon they would be deployed into production.

@HebaruSan HebaruSan requested a review from V1TA5 July 16, 2019 19:57
@HebaruSan HebaruSan changed the title From dev old theirs Pull forward old dev branch changes Jul 16, 2019
THANKS Outdated Show resolved Hide resolved
SpaceDock/app.py Outdated Show resolved Hide resolved
@DasSkelett
Copy link
Member

DasSkelett commented Jul 20, 2019

So I got the server running via the Docker image.
I've some changes needed for this branch to work ("work" in the sense of a running webserver and the ability to see the main page, there's most likely more needed to make it fully work). Should I do a PR to this branch? Of afterwards to the alpha branch?

Edit: there are more breaking changes regarding the template stuff in there. (At leased based on the current master branch, but since #198 this should also be the state of production if I'm not mistaken?)

More edit: Yep, with the redesign all the old templates were moved from /templates/* to templates/old/*, but since the paths are all hardcoded, quite a lot of jinja2.exceptions.TemplateNotFound are thrown, for nearly every action.

Note that I can't run this locally and I do not know how stable the old branch was, so it's possible that it breaks things. Close reading of the diffs may be advisable.

Looks like it was heavily WIP. The server won't even start up as is. Lots of sites missing. As I wrote above, I've got some fixes to get it running, but I think merging this whole PR depends on how @V1TA5 wants to proceed with the redesign, if he already wants to put it on the alpha server.

@DasSkelett
Copy link
Member

DasSkelett commented Jul 21, 2019

I wrote with @V1TA5 on the IRC. He doesn't want to continue this redesign/rewrite, instead do a more modern one.
That means that most commits of this PR shouldn't be merged.
However, other things apart from the redesign, that are 'complete' should be cherry-picked.

It's not easy to spot which changes are for other, incomplete features of the dev-branch and which could be useful for the current production code as is ('standalone'). Could have needed some squashes there.
But @V1TA5 agreed this one should be kept. Though I can imagine this also needs some work to fully function:

@HebaruSan
Copy link
Contributor Author

More edit: Yep, with the redesign all the old templates were moved vrom /templates/* to templates/old/*, but since the paths are all hardcoded, quite a lot of jinja2.exceptions.TemplateNotFound are thrown, for nearly every action.

Are the old templates intended to still be used at all? I'm trying to discern whether to approach this by updating the paths or switching to newer templates. Or maybe the usage of the newer templates was done already and then lost in a bad conflict resolution?

He doesn't want to continue this redesign/rewrite, instead do a more modern one.

Hmm. I don't like the idea of running a dev branch for X years as if it was going to be used and then summarily dropping it. Bad for morale; if contributors follow the current rules for submitting changes, their work should be incorporated one way or another. But since most of these changes were @V1TA5's, maybe morale is moot.

At the risk of contradicting myself, I also don't much like the idea of cherry-picking a few commits out of the middle of this mess. Who knows what assumptions and implicit dependencies there would be. Probably wiser to start fresh from how things are now and develop the desirable bits from scratch.

@V1TA5, what do you think of creating some enhancement issues to track the things you want to do? Sounds like item number one is rename KerbalStuff to SpaceDock?

@HebaruSan
Copy link
Contributor Author

I'm tired of resolving those conflicts over and over. Closing this in favor of fresh development, my apologies to anyone who contributed to the old dev branch for your lost work.

@HebaruSan HebaruSan closed this Jul 31, 2019
@DasSkelett
Copy link
Member

DasSkelett commented Jul 31, 2019

Are the old templates intended to still be used at all?

Not in the future, but this branch still relies on them since there isn't a new, redone template yet for every single page.

I'm trying to discern whether to approach this by updating the paths or switching to newer templates. Or maybe the usage of the newer templates was done already and then lost in a bad conflict resolution?

As I wrote above, there is not a corresponding new template for every old template yet (as far as I understood this branch and the changes).

But all this doesn't really matter, since @V1TA5 decided to discontinue the work on this redesign, and instead build up something completely different.

At the risk of contradicting myself, I also don't much like the idea of cherry-picking a few commits out of the middle of this mess. Who knows what assumptions and implicit dependencies there would be. Probably wiser to start fresh from how things are now and develop the desirable bits from scratch.

Closing this in favor of fresh development

Agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants