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

chore(procedures): merge procedures into one file #7196

Conversation

jniles
Copy link
Contributor

@jniles jniles commented Aug 13, 2023

This commit merges all the procedures into a single file, and deletes the old procedures/*.sql files. It also removes references to the different procedure files in our build steps.

Closes #7188.

This PR does not necessarily mean we need to go this direction the project maintainers do not wish to. It would just make things a bit easier when it comes to builds and deployment (though maybe increase in difficulty to maintenance).

@jniles jniles added the chore label Aug 13, 2023
@jniles jniles requested a review from jmcameron August 13, 2023 14:20
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

The code changes look fine. I tested it and it works.

However, please update the procedures.sql to have a separator (eg, horizontal bar) between each "file" in the procedures.sql indicating the nature of the following SQL code. For instance, if you look at line 690 or 1957 it is not clear what the description applies to. When it was in a separate file, it was obvious. But now that it is part of single file, it would be helpful to add a little header for each section with a separating line and a brief title about what the following section is about (possibly just a simple adaptation of the original SQL file name..)

Also, does anything need to be done in the SQL migration file?

@jniles
Copy link
Contributor Author

jniles commented Aug 23, 2023

Nothing needs to be done with the migration files.

I'll add in the requested changes this week. 👍 . I agree it would make it nicer.

@jniles jniles force-pushed the 7188-merge-stored-procedures-into-single-file branch from 689e0d9 to 6ed5ba5 Compare August 24, 2023 19:19
@jniles
Copy link
Contributor Author

jniles commented Aug 24, 2023

@jmcameron I believe the changes have been made!

This commit merges all the procedures into a single file, and deletes
the old procedures/*.sql files.  It also removes references to the
different procedure files in our build steps.

Closes IMA-WorldHealth#7188.
Adds better section headings to the procedures file.
@jniles jniles force-pushed the 7188-merge-stored-procedures-into-single-file branch from c22df40 to 8d1b1ad Compare August 24, 2023 20:40
Copy link
Collaborator

@jmcameron jmcameron left a comment

Choose a reason for hiding this comment

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

Thanks for the updates.

@jmcameron
Copy link
Collaborator

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 25, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 51a609d into IMA-WorldHealth:master Aug 25, 2023
3 checks passed
@jniles jniles deleted the 7188-merge-stored-procedures-into-single-file branch December 22, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merge all stored procedures from server/models/procedures/*.sql into server/models/procedures.sql
2 participants