Skip to content

Changed the names checking if they worked so full committing#24

Open
scarletnorberg wants to merge 2 commits intoFNAL-CTA:mainfrom
scarletnorberg:python_Tapepool_cta_admin
Open

Changed the names checking if they worked so full committing#24
scarletnorberg wants to merge 2 commits intoFNAL-CTA:mainfrom
scarletnorberg:python_Tapepool_cta_admin

Conversation

@scarletnorberg
Copy link
Contributor

No description provided.

Copy link
Collaborator

@ericvaandering ericvaandering left a comment

Choose a reason for hiding this comment

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

Some changes to comments. Lastly, open an issue on yourself somewhere (maybe the sprint) to refactor both cta_admin scripts to remove produce_prom_metric and move it to a shared location.

def produce_prom_metric(metric_name, metric_value, list_input, labels):
def produce_prom_metric(metric_name, metric_value, metric_input, labels):
# loop over labels to get key,value pairs
# print(len(labels))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a comment in the triple-double quotes at the beginning describing what this function does. It's called a docstring.

Then remove all the commented out code like print statements.

The comment # loop over labels... ... Generally you only want to describe code which is doing something not obvious from reading the code. So I leave it to you if you want to include it.

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