delta/gitlab/gitlab-ce.git - gitlab.com: gitlab-org/gitlab-ce.git

	Commit message (Collapse)	Author	Age	Files	Lines
*	Introduce "Set up autodeploy" button to help configure GitLab CI for deploymentadam-auto-deploy	Adam Niedzielski	2016-12-21	3	-2/+58
\| \| \| \| \| \|	The button allows to choose a ".gitlab-ci.yml" template that automatically sets up the deployment of an application. The currently supported template is Kubernetes template.
*	Merge branch 'fix/25843' into 'master'	Sean McGivern	2016-12-20	2	-2/+2
\|\ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Rename SlackNotificationService back to SlackService ## What does this MR do? See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8191#note_20310845 ## What are the relevant issue numbers? Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/25843 /cc @yorickpeterse @stanhu @smcgivern @felipe_artur @ayufan See merge request !8208
\| *	Rename SlackNotificationService back to SlackService	Douglas Barbosa Alexandre	2016-12-20	2	-2/+2
\| \|
* \|	Merge branch '19703-direct-link-pipelines' into 'master'	Grzegorz Bizon	2016-12-20	2	-9/+65
\|\ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Resolve "Direct link from pipeline list to builds" ## What does this MR do? - Adds a dropdown with builds in the mini pipeline graph in the pipelines table - Unnest a lot of CSS related with pipelines in order to make it reusable ## Screenshots ![Screen_Shot_2016-12-15_at_14.45.41](/uploads/ca1c61842a422a34383e029d668034b7/Screen_Shot_2016-12-15_at_14.45.41.png) ![Screen_Shot_2016-12-15_at_14.45.49](/uploads/952e3277143639ce4ad111103034faeb/Screen_Shot_2016-12-15_at_14.45.49.png) ![Screen_Shot_2016-12-15_at_14.46.02](/uploads/f7369a124b1c3c0db4194de2cb637ef0/Screen_Shot_2016-12-15_at_14.46.02.png) ![graph_animation](/uploads/9bae036cb5acff499f992a4722943d72/graph_animation.gif) ## Does this MR meet the acceptance criteria? - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - Tests - [x] Added for this feature/bug - [ ] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #25071 Closes #19703 See merge request !8097
\| * \|	Fix stage and pipeline specs and rubocop offenses19703-direct-link-pipelines	Grzegorz Bizon	2016-12-20	1	-8/+17
\| \| \|
\| * \|	Move test for HTML stage endpoint to controller specs	Grzegorz Bizon	2016-12-20	2	-33/+1
\| \| \|
\| * \|	Fix broken test	Filipa Lacerda	2016-12-20	1	-1/+1
\| \| \|
\| * \|	Fix broken test	Filipa Lacerda	2016-12-20	1	-1/+4
\| \| \| \| \| \| \| \| \| \| \| \|	Changes after review
\| * \|	Adds tests for the mini pipeline graph	Filipa Lacerda	2016-12-20	1	-3/+47
\| \| \|
\| * \|	Improve specs	Kamil Trzcinski	2016-12-20	1	-2/+5
\| \| \|
\| * \|	Add tests for stage API endpoint	Kamil Trzcinski	2016-12-20	1	-0/+29
\| \| \|
* \| \|	Merge branch 'jej-fix-n+1-queries-milestones-show' into 'master'	Sean McGivern	2016-12-20	1	-0/+26
\|\ \ \ \| \|_\|/ \|/\| \| \| \| \| \| \| \| \| \| \| \| \| \|	Fix N+1 queries on milestone show pages Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/25832 See merge request !8185
\| * \|	Added QueryRecorder to test N+1 fix on Milestone#showjej-fix-n+1-queries-milestones-show	James Edwards-Jones	2016-12-20	1	-0/+26
\| \| \|
* \| \|	Merge branch 'fix_team_member_expiration_date_spec' into 'master'	Grzegorz Bizon	2016-12-20	1	-2/+2
\|\ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Fix member with expiration date feature spec See merge request !8186
\| * \| \|	Fix member with expiration date feature specfix_team_member_expiration_date_spec	Felipe Artur	2016-12-19	1	-2/+2
\| \| \|/ \| \|/\|
* \| \|	Merge branch 'move-admin-projects-spinach-tests-to-rspec' into 'master'	Rémy Coutable	2016-12-20	1	-8/+91
\|\ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Move admin projects spinach tests to RSpec Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/23036 See merge request !8174
\| * \| \|	Move admin projects spinach tests to rspec	Semyon Pupkov	2016-12-19	1	-8/+91
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	https://gitlab.com/gitlab-org/gitlab-ce/issues/23036
* \| \| \|	Merge branch '22864-kubernetes-deploy-with-terminal' into 'master'	Kamil Trzciński	2016-12-20	2	-0/+60
\|\ \ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Add online terminal support for Kubernetes ## What does this MR do? Gives terminal access to kubernetes-deployed environments via the deployment service ## Are there points in the code the reviewer needs to double check? ## Why was this MR needed? Part of idea to production ## Screenshots (if relevant) ### `/root/reviewing/environments` ![Screen_Shot_2016-12-15_at_19.10.40](/uploads/bd2c54c07b6c85dec3328a20cd185b64/Screen_Shot_2016-12-15_at_19.10.40.png) ### `/root/reviewing/environments/10013` ![Screen_Shot_2016-12-19_at_12.52.39](/uploads/db4e4e06cda88437e8727433d65898b9/Screen_Shot_2016-12-19_at_12.52.39.png) ### `/root/reviewing/enviroments/10013/terminal` ![Screen_Shot_2016-12-15_at_02.35.52](/uploads/1bb77b7e2de2c657ae3bda62dc4f0970/Screen_Shot_2016-12-15_at_02.35.52.png) ## Does this MR meet the acceptance criteria? - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - Tests - [X] Added for this feature/bug - [x] All builds are passing - [X] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [X] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? * Closes #22864 #22958 * Alternative to, and somewhat based on, !6770 * Depends on https://gitlab.com/gitlab-org/gitlab-workhorse/merge_requests/83 See merge request !7690
\| * \| \| \|	Add terminal UI and controller actions	Fatih Acet	2016-12-19	2	-0/+60
\| \| \| \| \|
* \| \| \| \|	Merge branch 'zj-kamil-slack-slash-commands' into 'master'	Grzegorz Bizon	2016-12-20	2	-3/+43
\|\ \ \ \ \ \| \|_\|_\|/ / \|/\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Slack slash commands ## What does this MR do? Implement Slack Slash Commands by utilizing generalized Mattermost presenter to fulfill Slack requirements. ## Why was this MR needed? We want to expose Slack Slash Commands as a first-class service. ## What are the relevant issue numbers? Supersedes !8007 Closes #22182 See merge request !8126
\| * \| \| \|	Fix specszj-kamil-slack-slash-commands	Kamil Trzcinski	2016-12-19	1	-22/+14
\| \| \| \| \|
\| * \| \| \|	Improve after feedback	Kamil Trzcinski	2016-12-19	1	-9/+9
\| \| \| \| \|
\| * \| \| \|	Fix failures	Kamil Trzcinski	2016-12-18	1	-1/+1
\| \| \| \| \|
\| * \| \| \|	Fix spec failures	Kamil Trzcinski	2016-12-18	1	-2/+2
\| \| \| \| \|
\| * \| \| \|	Added slack slash commands frontend help well	Luke "Jared" Bennett	2016-12-18	1	-0/+48
\| \| \|_\|/ \| \|/\| \| \| \| \| \| \| \| \| \|	Added tests
* \| \| \|	Merge branch 'move-admin-appearance-feature-to-rspec' into 'master'	Rémy Coutable	2016-12-19	1	-0/+76
\|\ \ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Move Admin Appearance spinach feature to RSpec Part of #23036 See merge request !8063
\| * \| \| \|	Move Admin Appearance spinach feature to rspec	jurre	2016-12-19	1	-0/+76
\| \| \| \| \|
* \| \| \| \|	Merge branch 'pipeline-build-hitbox' into 'master'	Annabel Dunstone Gray	2016-12-19	1	-18/+15
\|\ \ \ \ \ \| \|_\|_\|/ / \|/\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Make CI badge hitboxes better match container ## What does this MR do? Makes the Pipeline graph badge hitboxes accurately match their containers. Currently, there's an issue where the badge highlights, as though, it's clickable, but clicking it does nothing: ![before](/uploads/668f841c640bc586044c89bed9f1e1e9/before.gif) This is due to the `<a>` and `<button>` elements being padded into the parent elements. So, the parent would react to a cursor, but the cursor wouldn't be on the `<a>`/`<button>` yet. ![before-hitbox](/uploads/6ebaaa4be1096fbfc37f8d73da492c7b/before-hitbox.png) ## Are there points in the code the reviewer needs to double check? Maybe the tests? I kept the style changes as minimal as possible. ## Why was this MR needed? I was annoyed every time I clicked on a badge that I was _clearly on_ (it highlighted), but nothing happened. If I found it irritating, perhaps someone else did too. ## Screenshots (if relevant) How it behaves in the PR: ![finished](/uploads/c26fabe37ccf17f171f40b48522702b2/finished.gif) ## Does this MR meet the acceptance criteria? - [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added - Tests - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #25547 See merge request !8156
\| * \| \| \|	Make CI badge hitboxes better match container	Mitchell Hentges	2016-12-19	1	-18/+15
\| \|/ / /
* \| \| \|	Move admin broadcast messages spinach feature to rspec	jurre	2016-12-19	1	-0/+51
\|/ / /
* \| \|	Merge branch 'move-admin-label-spinach-test-to-rspec' into 'master'	Rémy Coutable	2016-12-19	1	-0/+99
\|\ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Move admin labels spinach test to RSpec Part of #23036 See merge request !8166
\| * \| \|	Move admin labels spinach test to rspec	Semyon Pupkov	2016-12-19	1	-0/+99
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	https://gitlab.com/gitlab-org/gitlab-ce/issues/23036
* \| \| \|	Merge branch 'fix/detailed-status-groups' into 'master'	Lin Jen-Shin (godfat)	2016-12-19	1	-1/+1
\|\ \ \ \ \| \|_\|_\|/ \|/\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Improve icons and style for manual build status ## What does this MR do? This MR changes the CSS class for manual build status badge to make it a little more different than skipped build status badge. It makes it darker. ## Screenshots ![Screen_Shot_2016-12-17_at_13.37.58](/uploads/9628f0e84fdaad82d8833f546f0192a6/Screen_Shot_2016-12-17_at_13.37.58.png)![Screen_Shot_2016-12-17_at_13.31.48](/uploads/fe8e27df0274b9f9ae840110593a298a/Screen_Shot_2016-12-17_at_13.31.48.png) See merge request !8102
\| * \| \|	Fix CI/CD statuses actions' CSS on pipeline graphs	Grzegorz Bizon	2016-12-17	1	-1/+1
\| \| \| \|
* \| \| \|	Merge branch 'dockerfile-templates' into 'master'	Rémy Coutable	2016-12-19	1	-0/+30
\|\ \ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Allow to use Dockerfile templates See merge request !7247
\| * \ \ \	Merge remote-tracking branch 'origin/master' into dockerfile-templatesdockerfile-templates	Kamil Trzcinski	2016-12-18	105	-1681/+3883
\| \|\ \ \ \ \| \| \| \|/ / \| \| \|/\| \|
\| * \| \| \|	Refactored JS	Luke "Jared" Bennett	2016-11-08	1	-0/+30
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Added spec
* \| \| \| \|	Merge branch 'move-admin-application-spinach-test-to-rspec' into 'master'	Rémy Coutable	2016-12-19	1	-0/+36
\|\ \ \ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Move admin application spinach test to RSpec Part of #23036 See merge request !8140
\| * \| \| \| \|	Move admin application spinach test to rspec	Semyon Pupkov	2016-12-17	1	-0/+36
\| \| \|/ / / \| \|/\| \| \| \| \| \| \| \| \| \| \| \| \|	https://gitlab.com/gitlab-org/gitlab-ce/issues/23036
* \| \| \| \|	Move admin deploy keys spinach test to rspec	Semyon Pupkov	2016-12-17	1	-0/+29
\|/ / / / \| \| \| \| \| \| \| \| \| \| \| \|	https://gitlab.com/gitlab-org/gitlab-ce/issues/23036
* \| \| \|	Merge branch 'cleaner-merge-commit-messages' into 'master'	Sean McGivern	2016-12-17	2	-0/+129
\|\ \ \ \ \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Cleaner merge commit messages Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/23462 See merge request !7722
\| * \| \| \|	Change closes issues reference text on MergeRequest#merge_commit_message to ↵	Gabriel Gizotti	2016-12-16	1	-1/+1
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	match existing text generated by the system
\| * \| \| \|	add feature specs to test toggling of merge commit message between message ↵	Gabriel Gizotti	2016-12-16	2	-3/+75
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	with description and without
\| * \| \| \|	Add feature spec to verify all 4 different states of closing issues message ↵	Gabriel Gizotti	2016-12-16	1	-0/+57
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	on Merge Request show page.
* \| \| \| \|	Improve test for sort dropdown on members page	Nur Rony	2016-12-16	2	-0/+18
\| \| \| \| \|
* \| \| \| \|	Use factories to create project/group membership on specs	Douglas Barbosa Alexandre	2016-12-16	2	-4/+4
\| \| \| \| \|
* \| \| \| \|	Sort group/project members alphabetically by default	Douglas Barbosa Alexandre	2016-12-16	2	-0/+14
\| \| \| \| \|
* \| \| \| \|	Add feature spec for sort functionality on group/project members list	Douglas Barbosa Alexandre	2016-12-16	2	-0/+164
\| \| \| \| \|
* \| \| \| \|	Merge branch '18435-autocomplete-is-not-performant' into 'master'	Fatih Acet	2016-12-16	2	-5/+6
\|\ \ \ \ \ \| \|_\|_\|/ / \|/\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	Make autocomplete request performant ## What does this MR do? Restricts the `autocomplete_sources`action to returning the `json`of one specific `at` type at a time. Makes sure all commands start by immediately showing the `loading` dropdown. If the `loading` dropdown is active, we will request and load in the data for that specific `at` type. We manually trigger the filter again. This time, if `loading` dropdown is not active, initiate default filter behaviour and use the correct data template. Also fixes an issue where `setup` was called multiple times by only setting up and destroying atwho on a focused/blured input. ## Are there points in the code the reviewer needs to double check? ## Why was this MR needed? `autocomplete_sources` was requested on page load and is large. ## Screenshots (if relevant) 3 requests for 3 different first-time references. ![Screen_Shot_2016-10-14_at_14.43.12](/uploads/0b04eab0d0bf3638d6d6e431036b8089/Screen_Shot_2016-10-14_at_14.43.12.png) ## Does this MR meet the acceptance criteria? - [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added - [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md) - [ ] API support added - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html) - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if it does - rebase it please) - [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits) ## What are the relevant issue numbers? Closes #22802 Closes #18435 See merge request !6856
\| * \| \| \|	Changed autocomplete_sources into an action that returns a single 'at' type ↵18435-autocomplete-is-not-performant	Luke Bennett	2016-12-15	2	-5/+6
\| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \| \|	of sources at a time Finished up autocomplete_sources action and added frontend to fetch data only when its needed Added wait_for_ajax to specs Fixed builds and improved the setup/destroy lifecycle Changed global namespace and DRYed up loading logic Added safety for accidentally loading data twice Removed destroy as its not necessary and is messing with click events from a blur race condition Created AutocompleteSourcesController and updated routes Fixed @undefined from tabbing before load ends Disable tabSelectsMatch until we have loaded data Review changes