cleanup.rake typo fix: this -> these
Created by: bbodenmiller
Minor typo fix in cleanup.rake
.
Created by: TeatroIO
I've prepared a stage. Click to open.
By Administrator on 2014-11-15T11:17:41 (imported from GitLab project)
39 39 end 40 40 41 41 unless remove_flag 42 puts "To cleanup this directories run this command with REMOVE=true".yellow 42 puts "To cleanup these directories run this command with REMOVE=true".yellow 43 43 end 39 39 end 40 40 41 41 unless remove_flag 42 puts "To cleanup this directories run this command with REMOVE=true".yellow 42 puts "To cleanup these directories run this command with REMOVE=true".yellow 43 43 end Created by: bbodenmiller
@cirosantilli Also unrelated looks like
cleanup:repos
is incorrectly matching user namespacejohn.mangit
as a global directory to remove. Doesdir =~ /.git$/
need to be changed todir =~ /\.git$/
or similar?By Administrator on 2014-11-15T11:30:52 (imported from GitLab project)
Created by: cirosantilli
Maybe the double slash is at
all_dirs = Dir.glob(git_base_path + '/*')
. A quickDir.glob('./' + '/*')
on my IRB gives double slash paths, andrepos_path
does end in/
. Whenever I see explicit/
and+
to join paths it hurts my eyes.+ '*'
should work (not sure). Better useFile
orDir
to do this instead ofGlob
since glob is insane.By Administrator on 2014-11-15T11:31:51 (imported from GitLab project)
39 39 end 40 40 41 41 unless remove_flag 42 puts "To cleanup this directories run this command with REMOVE=true".yellow 42 puts "To cleanup these directories run this command with REMOVE=true".yellow 43 43 end Created by: cirosantilli
IIUC, you fixed this bug twice: here and on the other line which are the same.
Instead of fixing bugs twice, it is better to factor things out first.
Here I'd do both on the same PR. And this
unless
block is the same as the other one, so I'd create a new method for it.By Administrator on 2014-11-15T11:32:39 (imported from GitLab project)
Created by: cirosantilli
I don't know what that
dir =~ /.git$/
is used for. IIUC,repos_path
is/repositories
, soall_dirs
are only the namespaces, which cannot end in.git
. Likely what he meant is correct, to allow people to put bare repos at/repositories/repo.git
, but I don't know why anyone would do that, and I don't think gitlab does anything special with those. I'd just remove that check and see what jacob says. But if people did to that in the past, it might break them. In that case, there should be a comment on the source explaining it is there for historical reasons.By Administrator on 2014-11-15T11:39:30 (imported from GitLab project)
Created by: bbodenmiller
@cirosantilli previously GitLab used to have global projects so these are just cleanup tasks since old GitLab installs didn't necessary delete folders correctly after moving them. The issue I'm having is it appears it actually only looks to see if the folder name ends in
git
rather than checking if it ends in.git
.By Administrator on 2014-11-15T12:10:22 (imported from GitLab project)
Created by: cirosantilli
Ah, makes sense. Even better, just use
ends_with?
like in: https://github.com/gitlabhq/gitlabhq/pull/8096By Administrator on 2014-11-15T12:26:35 (imported from GitLab project)
Created by: jvanbaarsen
This merge request has been closed because a request for more information has not been reacted to for more than 2 weeks. If you respond and conform to the merge request guidelines in our contributing guidelines we will reopen this merge request.
By Administrator on 2015-01-18T18:09:55 (imported from GitLab project)