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

added caching script gen1 function in main file #435

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

octonawish-akcodes
Copy link
Contributor

Need reviews @spbnick and how to test it.

@octonawish-akcodes octonawish-akcodes changed the title added cacging script gen1 function in main file added caching script gen1 function in main file Jul 4, 2023
Copy link
Collaborator

@spbnick spbnick left a comment

Choose a reason for hiding this comment

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

This is a good start! Thank you @octonawish-akcodes.

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
kcidb/cache/__init__.py Show resolved Hide resolved
kcidb/cache/__init__.py Show resolved Hide resolved
kcidb/cache/__init__.py Outdated Show resolved Hide resolved
kcidb/cache/__init__.py Outdated Show resolved Hide resolved
kcidb/cache/__init__.py Show resolved Hide resolved
Copy link
Collaborator

@spbnick spbnick left a comment

Choose a reason for hiding this comment

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

Looks alright to me, thank you, Abhishek, but let's work on the commit message a bit.

First of all, please write your commit messages in imperative mood (as if giving a command). I.e. instead of writing "Added", please write "Add".

Then let's work on the meaning. Right now you're saying you've added a "script". "Script" usually means a quickly put-together (somewhat poorly-made) program, an executable, which you can run by itself. You're adding no such thing. What you're adding here is a module (kcidb.cache), a couple functions in another module (main.py), and their dependencies.

Additionally, you're saying that you're adding a "gen1 function". However, there's no such thing as a "gen1 function" in general, without the context of Google Cloud Functions, which you don't have in your message. So it would make little sense to whoever else reads that. Further on, "gen1" is just an implementation detail which has little practical effect, and is especially pointless to mention, since all our Cloud Functions are "gen1".

What I would write instead, is something along the lines of "Add URL cache client and a caching Cloud Function". This says that we're adding a "URL cache client", without going into details of how it's implemented (i.e. the module, as that's normally how it's done), and mentions the Cloud Function, as that's an important high-level interface, but skips specifying which generation it is, as that's not really important, and because we gotta keep our commit subjects short.

In a larger community, it would be important to elaborate the details and the purpose in the commit message body, but we all know what this is about, here.

@octonawish-akcodes
Copy link
Contributor Author

Ah!!! okay. I will remember this in future PR's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants