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

skip github repos #522

Closed
wants to merge 3 commits into from
Closed

skip github repos #522

wants to merge 3 commits into from

Conversation

andreacfm
Copy link

Add a new config to allow skipping github repos from the display.
Repos are skipped ny name.

@heptat
Copy link

heptat commented Sep 12, 2012

Looks like there's a bug here.

In github.js as you loop through repos (starting line 34), you (potentially) call repos.splice(i,1) (line 36)...but this removes an element from repos, reducing its length. If its length is reduced, you are never able to loop through all elements in repos.

I guess the solution is to use a new array and populate it with elements in repos that are not in your "skip" list.

Alternatively this pull request looks like it will work: #400 (ah, it's still using the old github v2 api...so easiest just to fix 522).

if(skips.length > 0){
for(var i=0; i < repos.length; i++){
if(skips.indexOf(repos[i].name) >= 0){
repos.splice(i,1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't call repos.splice inside of loop through repos.length because splicing (in this case removing an element) changes the length of repos, therefore not all elements will be iterated over.

@heptat
Copy link

heptat commented Sep 12, 2012

My solution is (from line 32):

var display_repos = new Array();                                                                                                                                                               
if(skips.length > 0){                                                                                                                                                                          
  for(var i=0; i < repos.length; i++){                                                                                                                                                         
    if(skips.indexOf(repos[i].name) < 0){                                                                                                                                                    
      display_repos.push(repos[i]);                                                                                                                                                        
    }                                                                                                                                                                                        
  }                                                                                                                                                                                            
}                                                                                                                                                                                              

render(options.target, display_repos);

Tested against all edge cases.

My javascript skills aren't flash, so there may well be a much nicer way to do this.

@parkr
Copy link
Collaborator

parkr commented Mar 5, 2013

We've made it so that you have to explicitly include them via your configuration file for the GH repos sidebar plugin. This is thus unneeded.

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

Successfully merging this pull request may close these issues.

3 participants