-
Notifications
You must be signed in to change notification settings - Fork 2
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
adds logic for gcn.notices. notices, replace healpix_file if present #162
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Replace all fields larger than a certain size, not just the
healpix_file
field. - Fix CI failures.
For the first point, is there a size you have in mind? Like are we talking a max string length? |
Yes. |
Any rough idea what the cutoff should be? 256, 1000, ? |
Let's start with 512 bytes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to traverse the entire JSON object, not just the top-level dictionary entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a lot of unrelated, non-functional, stylistic changes here. It looks like you ran a code formatter on this. Please do that in a separate PR that contains only the reformatting.
What are the linting settings for this project? My vscode keeps replacing stuff on save that I don't intend it to |
We don't have any. Tell you what, I'll make a PR to set up black. |
7969822
to
df2cc95
Compare
gcn_email/core.py
Outdated
Name='/RemixGcnProduction/tables/email_notification_subscription' | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undo non-functional formatting change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rebase to pick up black formatting.
Fixes formating and check size of all fields Adds function to traverse dict and replace content thats too long
Co-authored-by: Leo Singer <[email protected]>
1aac7ab
to
b15fceb
Compare
Some of my changes got removed in the rebase, fixing now |
No description provided.