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

[DS-1085] EPerson last_active field is defined but never filled #164

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
3 participants
@mwoodiupui
Copy link
Member

commented Jan 11, 2013

Fixing this facilitates the identification of long-unused accounts, which one might wish to remove or ask about.

@helix84

This comment has been minimized.

Copy link
Member

commented Jan 11, 2013

Please, DON'T merge this pull request. I'll rebase it for you.

@mwoodiupui

This comment has been minimized.

Copy link
Member Author

commented Jan 11, 2013

Is there a problem?

@helix84

This comment has been minimized.

Copy link
Member

commented Jan 11, 2013

Yes! This is just plain ugly: 89b578a

@helix84 helix84 closed this Jan 11, 2013

@tdonohue

This comment has been minimized.

Copy link
Member

commented Jan 11, 2013

I'm assuming the issue that helix84 is pointing at is the fact that there's an unnecessary "Merge" commit included here.

In order to get everyone better up-to-speed with best practices around when to rebase pull requests, could someone please get our Git docs updated around Pull Requests? https://wiki.duraspace.org/display/DSPACE/Development+with+Git#DevelopmentwithGit-ContributingChanges/PatchestoDSpaceviaGitHub

As it is, I feel like we're all merging/creating pull requests differently. Would be nice to clarify things in docs (even I'm a bit fuzzy at times in this area) & refer everyone to those docs.

@mwoodiupui

This comment has been minimized.

Copy link
Member Author

commented Jan 15, 2013

OK, git-noob question: where is the 'git amioutofsync' command? The branch had been out of touch with upstream for some months, and I needed a way to find out whether it had been clobbered by later work. My old SVN habits told me to pull updates from the repository before committing. What should I have done?

I'm not sure what harm the merge does? It should melt into air when merged back to upstream, since all of those commits are already in there, leaving nothing but a history record showing that someone was diligent.

I'll note that the referenced DSpace documentation, with respect to rebasing, essentially says "don't."

@helix84

This comment has been minimized.

Copy link
Member

commented Jan 16, 2013

git fetch origin # to connect to the remote repo and download info about latest branches and revisions
git status # will inform you how manny commits you are ahead or behind

alternatively, git pull origin master will also tell you that (pull = fetch + merge), or a git checkout preceded by git fetch.

There's really quite a lot written on merge vs. rebase, so I'll only address the specifics.

  1. The problem with your commit is that you merged master into a feature branch, essentially undoing all the interim work on the master branch and then putting it all back in one huge commit. Yes, it does compile and work, but it's just ugly (not to mention that it wastes some storage space, which in this case is quite small, but should be avoided in principle). In a normal workflow, you would only merge a feature branch to the master branch.
  2. It's true that they always say never rebase a public branch.
  • I sometimes do that, but only as a shortcut and when it's harmless - in my clone, never in the official repo; and only before merging, which means the feature branch will be thrown away anyway and I don't expect anyone to base any other branch off of it.
  • Note that I didn't do it in this case. I created a separate rebased branch in my repo and a new pull request. You could do the same with a new branch in your repo, completely safely.

I hope that explains it. If you have any questions, feel free to ask.

@mwoodiupui

This comment has been minimized.

Copy link
Member Author

commented Jan 16, 2013

So, maybe I didn't even begin properly. "origin" is mwoodiupui/DSpace, not dspace/DSpace. My topic branch branched off of the fork I did way back when, whose "master" gets updated once in a while.

Meanwhile, git status tells me how different the topic branch in my local repository is from the topic branch in my GitHub repository. It tells me whether I have commits not yet pushed.

The problem (which I stated incorrectly) remains: how do I discover conflicting commits between dspace/DSpace/master and ., so I can resolve them? Without merging upstream.

The answer seems to be: always rebase. Assuming "origin" is my GitHub repo, "upstream" is dspace/DSpace, and . cloned from "origin", I guess this would be:

git fetch upstream
git rebase upstream
# tidy up conflicts, 'git rebase --continue', repeat until clean
git push
# go push the pull request button

This is rather scary, in light of all the "rebase considered harmful" stuff out there, but it was the only command I could find which detects conflicts but operates on local rather than a remote.

@helix84

This comment has been minimized.

Copy link
Member

commented Jan 16, 2013

Let me split up your question into 2:

how do I discover conflicting commits between dspace/DSpace/master?

git diff --stat upstream/master mwoodiupui/DS-1085
git log upstream/master..mwoodiupui/DS-1085

But in this case I personally prefer gitk (or one of its many clones).

and ., so I can resolve them? Without merging upstream.

rebase is indeed the only thing I can think of, but maybe there's another solution I'm unaware of.
Anyway, why avoid rebase? If used correctly (read 2) in my previous comment), it's a very powerful tool. Remember that you can experiment with rebase in a new branch (branches are cheap!):

$ git branch
* DS-1085
  master
$ git branch DS-1085-rebased
$ git checkout DS-1085-rebased
Switched to branch 'DS-1085-rebased'
$ git rebase -i master

kosarko added a commit to kosarko/DSpace that referenced this pull request May 29, 2015

kosarko pushed a commit to kosarko/DSpace that referenced this pull request May 29, 2015

Merge pull request DSpace#176 from ufal/issue_164
create piwik_reports with fresh install DSpace#164

alanorth added a commit to alanorth/DSpace that referenced this pull request Jan 14, 2016

Merge pull request DSpace#164 from alanorth/5_x-ccafs-project-tags
input-forms.xml: Add more CCAFS project identifiers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.