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

URL-encode GitHub reference #38

Merged
merged 3 commits into from Nov 11, 2016
Merged

URL-encode GitHub reference #38

merged 3 commits into from Nov 11, 2016

Conversation

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 15, 2016

in install_github().

Use case:

install_github("krlmlr/ranger/ranger-r-package/ranger@b-#118-dimnames")

(The dash in #118 is the offender.)

Also affects devtools, CC @jimhester @hadley.

@codecov-io
Copy link

@codecov-io codecov-io commented Sep 15, 2016

Current coverage is 93.78% (diff: 75.00%)

Merging #38 into master will decrease coverage by 0.08%

@@             master        #38   diff @@
==========================================
  Files            23         23          
  Lines          1028       1030     +2   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            965        966     +1   
- Misses           63         64     +1   
  Partials          0          0          

Powered by Codecov. Last update 28c820e...9bb69f1

Loading

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Nov 11, 2016

Could you please review?

Loading

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Nov 11, 2016

Thanks, looks good, but you don't seem to use vlapply, so maybe we can remove that for now.

Loading

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Nov 11, 2016

Thanks!

Loading

@gaborcsardi gaborcsardi merged commit f5d819a into r-lib:master Nov 11, 2016
0 of 2 checks passed
Loading
@krlmlr krlmlr deleted the b-dash branch Jul 8, 2017
@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Jul 8, 2017

@gaborcsardi: Do you plan a CRAN release anytime soon? I tend to use the dash in branch names...

Loading

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Jul 9, 2017

@krlmlr I don't mind, but it depends on crancache now. :(

Loading

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Jul 9, 2017

I learned a lesson: Release to CRAN before adding a dependency :/

Loading

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Jul 9, 2017

I think that's a good lesson, but does not help this situation.

It is actually trivial to temporarily remove the dependency, let me do it quickly.

Loading

@gaborcsardi
Copy link
Contributor

@gaborcsardi gaborcsardi commented Jul 9, 2017

Just submitted to CRAN.

Loading

@krlmlr
Copy link
Member Author

@krlmlr krlmlr commented Jul 10, 2017

Awesome, thanks!

Loading

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

Successfully merging this pull request may close these issues.

None yet

3 participants