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

Rename variables to full name variables #127

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Rename variables to full name variables #127

wants to merge 1 commit into from

Conversation

ayylouu
Copy link

@ayylouu ayylouu commented Jul 23, 2021

Rename variables to full name variables #120

changed:

  • cid, chan_id, ccid --> channel_id
  • uid --> user_id

chk_upd(
fullname, teams.update_one({"chan_id": cid}, {"$push": {"chals": chan.id}})
fullname, teams.update_one({"channel_id": channel_id}, {"$push": {"chals": chan.id}})
Copy link
Contributor

Choose a reason for hiding this comment

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

These database operations (update_one, find_one, etc.) might in fact be an issue as this would break old CTFs before this change.

I would say we should keep the key names (chan_id, etc.) for now and rather only change the Python variable names.

A solution to this would be to implement a legacy checker that would search for objects that match for example chan_id or channel_id, that should solve that issue.

@mklarz
Copy link
Contributor

mklarz commented Jul 30, 2021

Thanks for the help, this help with reading and understanding the code whenever someone else decides to read through it.

Only issue is my comment above. As this bot is using MongoDB (NoSQL), it shouldn't take much to add a legacy operation that checks for example both chan_id and channel_id when looking for objects.

I can have a take a look at this when I have time if you are unable to.

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