[SQL] Adding directories to separate new patches from clean-up patches from Archives #3531

ridz1208 · 2018-03-02T23:37:22Z

This creates 2 new directories in the /SQL/ folder:

Cleanup_patches: Directory where release clean-ups should be made to. to replace putting SQL code in PHP tools scripts and/or commenting out clean-up commands in the release scripts (See README.md for more details)
New_patches: Directory where new SQL scripts go instead of putting them in Archives which should contain archives and not patches that have not yet been ran... This directory contains any and all new sql scripts no matter the branch (See README.md for more details)

ridz1208 · 2018-03-02T23:37:42Z

@driusan please review grammar :S

PapillonMcGill · 2018-03-05T13:06:54Z

SQL/Cleanup_patches/README.md

@@ -0,0 +1,3 @@
+This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.
+


Does that mean someone could include the following instruction in such cleanup patches?
ALTER TABLE ADD COLUMN ....

@PapillonMcGill they could include whatever they want as long as its not critical to the functioning of the code.

things that go here are things that usually are commented out in the release patches

added a line to explain

PapillonMcGill · 2018-03-05T13:11:12Z

SQL/Cleanup_patches/README.md

@@ -0,0 +1,3 @@
+This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.
+
+These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these.


What would happen if a project decide not to run those cleanup patches?

theoretically everything should still work fine in their codebase/database, these should only be clean-up

added a line to explain

driusan

as requested

driusan · 2018-03-05T14:58:45Z

SQL/Cleanup_patches/README.md

@@ -0,0 +1,3 @@
+This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.


Can we call it CleanupPatches instead of Cleanup_patches? I don't like the mixed case and underscore.

s/as a/for/

s/clean-up/cleanup/

i was just being consistent with Release_patches.

I dont mind the change if you approve having Release_patces and CleanupPatches in the same directory

driusan · 2018-03-05T14:59:52Z

SQL/Cleanup_patches/README.md

@@ -0,0 +1,3 @@
+This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.
+
+These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these.


"of the patches" or "of these patches"

I think this is also part of the first paragraph and not a second paragraph too

I like "of these patches"

Another option is to just leave off the "of these"

driusan · 2018-03-05T15:01:27Z

SQL/New_patches/README.md

@@ -0,0 +1,3 @@
+This directory contains SQL patches necessary during the upgrade of loris. Upon release, the SQL patches concerned will be concatenated into a single release patch stored under the `SQL/Release_patches` directory. The files used for the concatenation are then moved to the `SQL/Archive/X.Y` directory.


same comment about the directory name, and LORIS should be uppercase.

ridz1208 · 2018-03-05T18:17:47Z

@driusan @PapillonMcGill please re-review

driusan · 2018-03-05T18:18:01Z

SQL/Cleanup_patches/README.md

-This directory contains SQL patches written for post-release cleanup. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.
-
-These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these patches.
+This directory contains SQL patches written for post-release cleanup. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements. These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these patches. The content of these patches should not be mandotory for proper functioning of the database, they should be run for the sole purpose of removing unused tables, columns or fields; any mandatory cleanup should be added to the release patches.


PapillonMcGill · 2018-03-05T18:37:31Z

SQL/Cleanup_patches/README.md

@@ -0,0 +1 @@
+This directory contains SQL patches written for post-release cleanup. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements. These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these patches. The content of these patches should not be mandotory for proper functioning of the database, they should be run for the sole purpose of removing unused tables, columns or fields; any mandatory cleanup should be added to the release patches.


You mention DELETE statement but near the end of the paragraph, you mention removing columns or fields, not rows.

Could you add precision about when DELETE could be used? or remove DELETE statement?

rows should be there as well, adding it to the readme

Directories created

95d12c0

ridz1208 added Cleanup PR or issue introducing/requiring at least one clean-up operation [branch] major labels Mar 2, 2018

ridz1208 requested review from driusan, xlecours and PapillonMcGill March 2, 2018 23:37

ridz1208 mentioned this pull request Mar 2, 2018

[Candidate_parameters, Core] Normalizing Consent SQL 1/2 #3532

Merged

4 tasks

PapillonMcGill suggested changes Mar 5, 2018

View reviewed changes

PapillonMcGill added the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Mar 5, 2018

driusan reviewed Mar 5, 2018

View reviewed changes

ridz1208 added 3 commits March 5, 2018 13:13

Update README.md

93b72d5

added clause

3d7a87f

Update README.md

0ee38af

driusan reviewed Mar 5, 2018

View reviewed changes

PapillonMcGill reviewed Mar 5, 2018

View reviewed changes

Update README.md

39769e3

PapillonMcGill removed the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Mar 5, 2018

PapillonMcGill approved these changes Mar 12, 2018

View reviewed changes

driusan merged commit 88b4b32 into aces:major Mar 12, 2018

ridz1208 added this to the 20.0 milestone Apr 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SQL] Adding directories to separate new patches from clean-up patches from Archives #3531

[SQL] Adding directories to separate new patches from clean-up patches from Archives #3531

ridz1208 commented Mar 2, 2018

ridz1208 commented Mar 2, 2018

PapillonMcGill Mar 5, 2018

ridz1208 Mar 5, 2018

ridz1208 Mar 5, 2018

PapillonMcGill Mar 5, 2018

ridz1208 Mar 5, 2018

ridz1208 Mar 5, 2018

driusan left a comment

driusan Mar 5, 2018

driusan Mar 5, 2018

driusan Mar 5, 2018

ridz1208 Mar 5, 2018

driusan Mar 5, 2018

driusan Mar 5, 2018

PapillonMcGill Mar 5, 2018

driusan Mar 5, 2018

driusan Mar 5, 2018

ridz1208 commented Mar 5, 2018

driusan Mar 5, 2018

PapillonMcGill Mar 5, 2018

ridz1208 Mar 5, 2018

		@@ -0,0 +1,3 @@
		This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.

		@@ -0,0 +1,3 @@
		This directory contains SQL patches written as a post-release clean-up. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements.

		These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these.

		@@ -0,0 +1,3 @@
		This directory contains SQL patches necessary during the upgrade of loris. Upon release, the SQL patches concerned will be concatenated into a single release patch stored under the `SQL/Release_patches` directory. The files used for the concatenation are then moved to the `SQL/Archive/X.Y` directory.

		@@ -0,0 +1 @@
		This directory contains SQL patches written for post-release cleanup. These patches can contain `ALTER TABLE`, `DELETE` and/or `DROP` statements. These patches should be run at the discretion of the projects with the full knowledge of the possible side effects of these patches. The content of these patches should not be mandotory for proper functioning of the database, they should be run for the sole purpose of removing unused tables, columns or fields; any mandatory cleanup should be added to the release patches.

[SQL] Adding directories to separate new patches from clean-up patches from Archives #3531

[SQL] Adding directories to separate new patches from clean-up patches from Archives #3531

Conversation

ridz1208 commented Mar 2, 2018

ridz1208 commented Mar 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

driusan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ridz1208 commented Mar 5, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment