search

jtr13 / cc21fall1

Source files for EDAV Fall 2021 Mon / Wed Community Contribution Project
https://jtr13.github.io/cc21fall1
4 stars 67 forks source link

Icons and symbols in r #24

Closed celiajin closed 2 years ago

celiajin commented 2 years ago

Switch to the right branch.

ghost commented 2 years ago

@celiajin Thank you for your contribution!

ghost commented 2 years ago

@jtr13 Hi Dr. Robbins,

I updated the _bookdown.yml file in the PR branch. However, I am not getting the option to resolve conflicts. I don't want to merge this pull request because I'm not sure if it will overwrite the current _bookdown.yml for the entire project. Do you know how to proceed? Thank you!

jtr13 commented 2 years ago

Yeah, we need to modify the process to deal with this case. @s10singh97 can you advise on how to handle now based on what we did after class yesterday?

s10singh97 commented 2 years ago

Yes Professor! Instead of removing all the names from _bookdown.yml, copy the latest version of _bookdown.yml from the main branch of our project (it will contain the updated and the latest version of the _bookdown.yml file) and paste it into the submitted repo's _bookdown.yml file and at the end you can add the name of the new .Rmd file. In this way there would be no conflicts and we can just merge it directly. Alternatively, once you have copied the main branch bookdown.yml file in the student's branch and added their file name, you can mark it for review and I can review it.

s10singh97 commented 2 years ago

@mtz2110 In case of any confusion, please do tag me before merging and I will look into it. Thanks!

ghost commented 2 years ago

@s10singh97 it seems to have been fixed and ready to merge now!

jtr13 commented 2 years ago

@s10singh97 If you don't mind could you share this info with your counterparts in the other section? Either by email or by starting a discussion on their repo jtr13/cc21fall2 works...

ghost commented 2 years ago

@s10singh97 do you know what we should do about this pull request. I don't think there is a problem in merging it but it was also commented out in the bookdown.

s10singh97 commented 2 years ago

@mtz2110 They are loading an additional font file named as wmpeople1.TTF on line 84. The build failed because it was not able to find this file. I think that this contribution was not ready to get merged in the main branch.

As mentioned on the 7th bullet point here - Links to internal files must contain resources/<project_name>/ in the path. Also, I would recommend you to please go through all the criteria's thoroughly before merging.

I have also updated the new merge steps here.

An additional step after merge is to check the github actions page to check whether it is able to completely render the github page or is there some problem. Actions may take some time to completely render the project.

In case of any doubts, please feel free to tag me.

Thanks!

ghost commented 2 years ago

@s10singh97 Thanks for pointing that out. I looked over the .Rmd file a couple of times but missed that.

@celiajin can you open a new pull request and add the wmpeople1.TTF file in this format: Links to internal files must contain resources// in the path. Thanks!