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

*.scss: fix typos #1644

Closed
wants to merge 4 commits into from
Closed

*.scss: fix typos #1644

wants to merge 4 commits into from

Conversation

the-j0k3r
Copy link

@the-j0k3r the-j0k3r commented May 6, 2018

There are probably more but this is as far I I was.

edit

there is this @ line 201

padding: 0.7em em 0.7em 0.5em;
introduced in #1520

Should be a value there no idea what it was intended for that em if I had to guess Id say 0.5em
if you wish, I can push that in here also.

@the-j0k3r the-j0k3r changed the title profile.scss: fix typo in .profile-repositorylist selector *.scss: fix typos May 6, 2018
@backspace
Copy link
Contributor

Hey, thanks for this. I have a giant PR that’s awaiting cleanup and merging and I don’t want to introduce any merge conflicts, so I’ll look at this again after that‘s done.

@the-j0k3r
Copy link
Author

the-j0k3r commented May 7, 2018

No problem, I stumbled across this while making a dark style for travis-ci based on the generated css.

Where is that PR of yours? Ide like to have a look, I dread to have spent hours on making a dark style that wont work after that... 👅 💀 🔫

@backspace
Copy link
Contributor

Sorry, I should have linked to it. It’s here, it’s the updates for GitHub Apps support and the displaying of open source repositories at travis-ci.com. It’s my plan this week to break it up into some more-manageable parts and merge them.

@the-j0k3r
Copy link
Author

the-j0k3r commented May 7, 2018

That looks trivial tbh, only the profile.scss file 6d67800 and 6d67800 is affected by your PR, and that is minimal, you should have no issues.

That said, I found loads of other issues which Im unsure about, like using floatwith display: inline-block and other warnings like padding: inherit 0 idk what the 0 is doing there but causes errors, and many such others.

@backspace
Copy link
Contributor

heh well it wasn’t trivial to me, it’s been the major work of the past month.

There are a lot of problems within the CSS, as you’ve found. I can’t prioritise addressing them at the moment, sadly.

@the-j0k3r
Copy link
Author

the-j0k3r commented May 9, 2018

heh well it wasn’t trivial to me, it’s been the major work of the past month.

I meant the only conflict file is only two lines maybe even just one, The rebase operation should handle it automatically and if you use Atom its shouldn't make much noise if it does.

I didnt mean the work you did was trivial :D that trivial definitely is not sir, most certainly not.

Ill rebase this off of master anyway, thanks for your time sir.

@the-j0k3r
Copy link
Author

@backspace there we go all done and rebased against master.

backspace added a commit that referenced this pull request May 9, 2018
Thanks to @the-j0k3r for opening #1644.
@backspace
Copy link
Contributor

I ended up adding another commit in #1656 as the display: flex was a mistaken inclusion. Thanks for finding these!

@backspace backspace closed this May 9, 2018
@the-j0k3r
Copy link
Author

Not very nice to remove my contribution in this manner.

@backspace
Copy link
Contributor

I mentioned you in the squashed commit and linked to your PR, I don’t understand how that’s a removal. Do you have a recommendation on what I could have done differently?

My recommendation to you is to allow edits from maintainers, I wouldn’t have had to make my own PR that way.

@the-j0k3r
Copy link
Author

the-j0k3r commented May 10, 2018

I mentioned you in the squashed commit and linked to your PR

Not same thing according to how GitHub tracks contributions but... OK...

Do you have a recommendation on what I could have done differently?

Yes.

My recommendation to you is to allow edits from maintainers, I wouldn’t have had to make my own PR that way.

You could have asked or made a review/comment or something, that's what these issue thinga-me-bobs are for no?

👍

While we are here.. were are the repos for Travis Status and Travis.com, I found an inordinate amount of issues, typos similar to these, dozens of duplicate properties and lord knows what else.

@backspace
Copy link
Contributor

Was your goal to get onto the contributor list on GitHub? If so, you could make an empty commit PR and I’ll merge it.

I have a limited amount of time to engage in this kind of work and I didn’t want to wait for your response.

@joshk
Copy link
Contributor

joshk commented May 10, 2018

@the-j0k3r your help and contributions are really appreciated. Thank you!

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