Prettify all the things (part 3)
What does this MR do?
Prettifies every module within:
app/assets/javascripts/pages
What are the relevant issue numbers?
gitlab-ce#52764
Does this MR meet the acceptance criteria?
-
Changelog entry added, if necessary -
Documentation created/updated -
Tests added for this feature/bug -
Conforms to the code review guidelines -
Conforms to the merge request performance guidelines -
Conforms to the style guides -
Conforms to the database guides
Merge request reports
Activity
2 Errors d2ee2eab: The commit subject must contain at least three words d2ee2eab: Commits that change 30 or more lines in more than three files must describe these changes in the commit body 2 Warnings This merge request is quite big (more than 1781 lines changed), please consider splitting it into multiple merge requests. This merge request changed files with disabled eslint rules. Please consider fixing them. Commit message standards
One or more commit messages do not meet our Git commit message standards. For more information on how to write a good commit message, take a look at How to Write a Git Commit Message.
Here is an example of a good commit message:
Reject ruby interpolation in externalized strings When using ruby interpolation in externalized strings, they can't be detected. Which means they will never be presented to be translated. To mix variables into translations we need to use `sprintf` instead. Instead of: _("Hello #{subject}") Use: _("Hello %{subject}") % { subject: 'world' }
This is an example of a bad commit message:
updated README.md
This commit message is bad because although it tells us that README.md is updated, it doesn't tell us why or how it was updated.
Disabled eslint rules
The following files have disabled
eslint
rules. Please consider fixing them:app/assets/javascripts/pages/dashboard/todos/index/todos.js
app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors.js
app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js
app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_util.js
app/assets/javascripts/pages/projects/network/network.js
app/assets/javascripts/pages/sessions/new/username_validator.js
Run the following command for more details
node_modules/.bin/eslint --report-unused-disable-directives --no-inline-config \ 'app/assets/javascripts/pages/dashboard/todos/index/todos.js' \ 'app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors.js' \ 'app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_graph.js' \ 'app/assets/javascripts/pages/projects/graphs/show/stat_graph_contributors_util.js' \ 'app/assets/javascripts/pages/projects/network/network.js' \ 'app/assets/javascripts/pages/sessions/new/username_validator.js'
Generated by
DangerEdited by 🤖 GitLab Bot 🤖assigned to @timzallmann
@mikegreiling I can confirm that this doesn't happen on master but happens on this branch. Assigning this back to you.
assigned to @mikegreiling
And also on part 7. I think these MRs are not branching off from one of the part MR branches. @mikegreiling what could be the wrong here? WDYT?
Edited by Fatih Acet@mikegreiling I checked out to part 8 and give it a try on my local. It was working fine so I merged it but I see the same error on part 3, 4 and 7.
Strange, though I think the difference with parts 8 and 1 were that I recently rebased them on
master
. Perhaps this is a bug which was fixed recently, but was part of the branch when it was created (these were all made at the same time).I'll see if I can replicate the issue and check whether rebasing fixes it
added 226 commits
-
680e3a89...d87e88a6 - 225 commits from branch
master
- d2ee2eab - Prettify app/assets/javascripts/pages
-
680e3a89...d87e88a6 - 225 commits from branch
@fatihacet the failure you were noticing is explained here: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22299#note_108351827
Likely running
yarn install
again would resolve it, but I've rebased the branches for parts 3, 4, and 7 anyway.assigned to @fatihacet
assigned to @timzallmann
Thanks a lot @mikegreiling, tried the changes and looking at them I would say LGTM.
mentioned in commit a97e599a
mentioned in issue gitlab-org/release/tasks#515 (closed)