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

Clean less files #6921

Merged
merged 3 commits into from
May 13, 2019
Merged

Clean less files #6921

merged 3 commits into from
May 13, 2019

Conversation

xf-
Copy link
Contributor

@xf- xf- commented May 12, 2019

  • replace tabs with spaces
  • remove unit from 0 values
  • add empty lines between selectors
  • split important with a space in all occurrences
  • combine margin: 0 with margin-top, -bottom, left, right to one line.

(And maybe add a linter afterwards, new code is more unified, easier to understand)

@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label May 12, 2019
@techknowlogick techknowlogick added this to the 1.9.0 milestone May 12, 2019
@techknowlogick
Copy link
Member

I think some of the empty lines were removed in a recent PR by @lafriks

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 12, 2019
@xf-
Copy link
Contributor Author

xf- commented May 12, 2019

@techknowlogick this is most common usage. easy to find a selector.

No empty lines if no selector/ attribute is in between. eg.

  }
}

i try to get an linter into lessc - it has an plugin (i mostly use sass and a gulp build pipeline). Source map is the next goal. Is only loaded if Development console is open

Btw:
Current, but single selector per line is a nice rule

footer {
	.ui.container .left, .ui.container .right {
		@media only screen and (max-width: 880px) {
			display: block;
			text-align: center;
			float: none;
		}
	}
}

Single line (and max-depth to avoid very large selectors):

footer {
    .ui {
        &.container {
            .left,
            .right {
                @media only screen and (max-width: 880px) {
                    display: block;
                    text-align: center;
                    float: none;
                }
            }
        }
    }
}

@codecov-io
Copy link

codecov-io commented May 12, 2019

Codecov Report

Merging #6921 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6921      +/-   ##
==========================================
+ Coverage   41.38%   41.38%   +<.01%     
==========================================
  Files         440      440              
  Lines       59703    59703              
==========================================
+ Hits        24709    24710       +1     
  Misses      31755    31755              
+ Partials     3239     3238       -1
Impacted Files Coverage Δ
modules/log/event.go 64.46% <0%> (-1.53%) ⬇️
modules/log/router.go 92.5% <0%> (+2.5%) ⬆️
modules/log/log.go 71.42% <0%> (+2.52%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0b78548...5cc3db2. Read the comment docs.

@techknowlogick
Copy link
Member

Sounds good. If you could get that linter in here as well that'd be awesome.

Thanks for all your contributions recently.

@lafriks
Copy link
Member

lafriks commented May 12, 2019

@techknowlogick nope, I also added some newlines :) But only in one file that I was changing

@techknowlogick
Copy link
Member

@lafriks ah ok. I just remembered something about empty lines. Thanks for clarification.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 12, 2019
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 13, 2019
@zeripath zeripath merged commit 6fb58a8 into go-gitea:master May 13, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants