Patch minor issues for fresh installs #7
Conversation
When the package is being installed, especially for the first time, importing the __version__ from the `makenote` package can result in `PackageNotFoundError`. So it's better to not dynamically fetch versions using import.
+ Since `cur.fetchone()` or `metadata_encoded` can potentially return None, they should be checked if they are not `None` before inferring them at index 0. + Use 'WHERE' instead of 'LIMIT', since 'LIMIT' clause is not valid for UPDATE syntax.
+ Use `database_connection` to replace the repetitive use of `get_connection`: Reduce the risk of leaving connections open unintentionally + Comparison to `bool` should be done using `is` + Abide by PEP 8: remove unused variables, f-strings, unused packages
+ Comparison to `bool` should be done using `is` + Abide by PEP 8: f-strings, unused packages
-> Rename `diaryfiledir` to `diaryFileDir` to make the diary path consistent across all files
Mamdasn
changed the title
|
سلام. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
ممنون از جواب زود هنگامتان. |
|
گمان می‌کردم دیدگاه رو پاک کرده‌ام :))
منظورم بخش گرفتن ورژن بود. این برای گرفتن متغیر همانطور که گفتم می‌خواستم این دیدگاه رو پاک کنم. |
|
خب بررسی کردم. تغییرات عالی بودند. فقط چند نکته: ۲. در بخش migrations چک می‌کنی که فایل دیتابیس خالی نباشه. این خیلی خوبه و مانع کرش کردن می‌شه. فقط یک نکته. در صورتی که فایل دیتابیس خالی باشه یک پیام خطا چاپ می‌کنی. این کار لازم نیست. میک‌نوت سوییچ create داره برای ایجاد یک دیتابیس خالی. دیتابیس خالی وقتی می‌تونه مهاجرت کنه نیازی به چاپ پیغام خطا نداره. همچنین در تابع دیگری در همون فایل چک می‌کنی که آیا متادیتا داره یا نه. باز هم خیلی کار خوبیه و باز هم پیغام خطا لازم نیست. این پیام زمانی چاپ می‌شه که یک فایل دیتابیس (یا فایلی که آخر اسمش .db داره) در مسیر دیتابیس‌های میک‌نوت قرار داشته باشه که مال میک‌نوت نباشه. در این صورت اولاً که میک‌نوت مشکلی با این فایل نداره و ازش استفاده نمی‌کنه. ثانیاً که شاید یه نفر به هر دلیلی فایل‌های دیگری در اون مسیر داره. (مثلاً شاید همهٔ فایل‌های دیتابیس همهٔ نرم‌افزارهایش رو توی یک پوشه نگهداری می‌کنه) در این صورت منطقی نیست که در هر به‌روزرسانی میک‌نوت یک پیغام خطا دریافت کنه. پس لطفاً این دو تا print رو از این فایل حذف کن. (در کل تنها اصلاحی که لازمه حذف چند تا پرینت است. 😃) سپاس فراوان! |
- Error prints - sql_to_csv unnecessary print
"There could be other application files in the same dir as the `makenote` db files"
|
با تشکر از نگاه تیزتان به تغییرات ایجاد شده. همانطور که پیشنهاد دادید, تغییرات زیر روی کد ایجاد شد:
|
diaryfiledirtodiaryFileDirto make the diary path consistent across all filesdatabase_connectionto replace the repetitive use ofget_connectionindbmanager: Reduce the risk of leaving sqlite connections open unintentionally & properly communicating withsqlitesetup.pydirectly frommakenote/__init__.pyinstead of importing themakenotepackage.cur.fetchone()ormetadata_encodedinmigrations.pycan potentially return None, they should be checked if they are notNonebefore inferring them at index 0.boolshould be done usingis