search

CRLSCSClub / CRLSTime

A clock for showing the CRLS schedule during the school day
6 stars 10 forks source link

Added Local Storage abilities for lunch #13

Closed maxtkc closed 7 years ago

maxtkc commented 7 years ago

I am requesting that you pull this branch. It is very good. It should work. It saves the lunch to document.localStorage

maxtkc commented 7 years ago

I think that I need edit access to push to this branch. I did make some changes that fix this bug.

maxtkc commented 7 years ago

This link explains the various permission levels: https://help.github.com/articles/repository-permission-levels-for-an-organization/ What you could do is give write permissions to people and then protect the branches that are created so that they can only be merged by you. You could protect master as well.

This is only available to Admin and Owner as shown in the table:

Merge pull requests on protected branches, even if there are no approved reviews

dougmcg commented 7 years ago

Max, I think you should have the access you need now to commit new code to this branch... yes?

maxtkc commented 7 years ago

I made the changes to line 274, although I pushed under the wrong username again... that's my other account, I think that these permissions are working well.

dougmcg commented 7 years ago

Did you decide not to use "===" ?

dougmcg commented 7 years ago

Max - I'd like to merge this but there are a couple issues. Two binary files were added in your commit. I might be able to delete them, but I can't figure out how. Can you remove them from the commit? Also, can you revise your most recent commit so it uses "===" instead of "==". I think we want the triple version, yes?

maxtkc commented 7 years ago

When this is merged with the other branches, I think that the changing the == to === will carry over, as long as there are no issues with merging due to changing the same part of the code in different branches.

dougmcg commented 7 years ago

So you are thinking that your new lines 277, 279, and 281 will be converted to using triple equal signs?

scornman commented 7 years ago

It looks to me like the getSchedule method is new in this commit, so the merge won't affect the == inside it. The == in the parts of the code you didn't change will be fine, but you will need to change the ones you moved into the getSchedule method, since git probably isn't smart enough to account for that.

EDIT: Sorry for the duplicate comment.

maxtkc commented 7 years ago

How do we resolve the conflicts? I don't exactly understand what the conflict is.