Skip to content

Conversation

@lanbinleo
Copy link
Contributor

No description provided.

Copy link
Owner

@BernieHuang2008 BernieHuang2008 left a comment

Choose a reason for hiding this comment

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

approve some changes, some pending.

Copy link
Owner

@BernieHuang2008 BernieHuang2008 left a comment

Choose a reason for hiding this comment

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

good

default_value = None


# 支持 type_ 为 [str, "默认值"] 的写法
Copy link
Owner

Choose a reason for hiding this comment

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

I suggest that to use tuple instead of list, since there's already a usage:

tb['col_name'] = str, "hello"

The code is as below
https://github.com/BernieHuang2008/MercurySQL/blob/ffc6e463bec44a94546b2438ab5c5474a5698d4b/MercurySQL/gensql/table.py#L148C1-L152C29

Python will parse the situation above as tuple, so it's recommend to have the same form as the one already implemented in table.__setitem__().

Copy link
Owner

Choose a reason for hiding this comment

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

@lanbinleo 这块最好还是你来改一下吧,我不知道你在哪调用了这玩意

Copy link
Owner

Choose a reason for hiding this comment

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

image
你看这一段


if name in self.columns:
if type_.lower() != self.columnsType[name].lower():
if rebuild:
Copy link
Owner

Choose a reason for hiding this comment

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

currently deciding whether i will use this ...

return str(data).lower()
elif isinstance(data, (int, float)):
return str(data)
elif isinstance(data, bytes):
Copy link
Owner

Choose a reason for hiding this comment

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

waiting to be tested

@BernieHuang2008
Copy link
Owner

@lanbinleo 这次改的有点多,我要先理解一下 (>_<)

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