Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Conversation

@simurai
Copy link
Contributor

@simurai simurai commented Dec 15, 2016

This PR renames the namespace of all the CSS classes from git to github. So that it matches the package name.

Conflicts

This PR probably conflicts with:

CI fail

Unrelated? ESLint found too many warnings (maximum: 0).

Copy link
Contributor

@smashwilson smashwilson left a comment

Choose a reason for hiding this comment

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

Looking good 👍

I tracked down the CI failure, but I'd ❤️ to get this merged quickly so that we can deal with conflicts in our other branches sooner rather than later.

Isn't it fun when we all work on sweeping changes that touch the entire package at once 😉

onclick={this.commit.bind(this)}
disabled={!this.isCommitButtonEnabled()}>{this.commitButtonText()}</button>
<div ref="remainingCharacters" className={`git-CommitView-remaining-characters ${remainingCharsClassName}`}>
<div ref="remainingCharacters" className={`github-CommitView-remaining-characters ${remainingCharsClassName}`}>
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this line is just over the maximum line length for eslint:

> eslint --max-warnings 0 test lib
/Users/travis/build/atom/github/lib/views/commit-view.js
112:1  warning  Line 112 exceeds the maximum line length of 120  max-len

@smashwilson smashwilson merged commit 9ce85a1 into master Dec 15, 2016
@smashwilson smashwilson deleted the sm-rename-to-github branch December 15, 2016 13:40
@simurai
Copy link
Contributor Author

simurai commented Dec 15, 2016

Isn't it fun when we all work on sweeping changes that touch the entire package at once

Yeah, sorry. 😇 Seeing you talk about renaming the keybindings reminded me to also rename the CSS classes.

Something I was thinking.. if we want to separate the styling more from the logic, we could use a convention to never find elements by its CSS class. Instead use a separate class with a js- prefix or an attribute, like ref=. Well, maybe it's fine as is. Classes shouldn't change that often.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants