ArtitalkJS / Artitalk

通过leancloud实现的可实时发布说说的js
https://artitalk.js.org
MIT License
329 stars 52 forks source link

[Closed] 变量命名污染与命名规范 #43

Closed ChrAlpha closed 4 years ago

ChrAlpha commented 4 years ago

目前为止,Artitalk 采用全局变量存储配置项、语言库以及执行过程中的部分变量。像是 appId 这些稍容易产生歧义一点的变量可能会与其他模块冲突,也就是命名污染。

为此,此 PR 将所有配置项移至 artitalkConfig 下(例如:appID => artitalkConfig.appID)、将语言库移至 langbase 下(例如:text0 => langbase.text0)、将执行过程中使用的全局变量移至 artitalkVar 下(例如:artitalk_emoji => artitalkVar.artitalk_emoji) 。旨在修复全局变量带来的命名污染问题。

除此以外,为使配置项更容易理解,此 PR 还规范了部分配置命名:


也许这次修改量较大,但无非就是做了上面两件事情。

审核同学请着重关注

  1. 注意有无 typo
  2. 本次 PR 不涉及功能变动,但注意是否影响现有特性
  3. 注意是否还有疏漏的全局变量没有处理
  4. 请尽量保持跟进直至此 PR 被 merged 或 closed

经初步测试,基础功能如发表说说、头像、翻页、背景、颜色的配置项均正常工作,深入功能如媒体资源上传等暂时没有能力测试。

此次 PR 难免会带来 breaking changes,需要一定时间测试,即便通过也还需要讨论版本号等后续事宜。这里就不 @ 成员了,毕竟也不像 上一个 PR 能够很快判断是否应该合并,大家随意在空闲时间测试即可。


如何测试

拉取至本地:

git clone -b master https://github.com/ChrAlpha/Artitalk.git
cd Artitalk

测试 dist/ 中的版本即可。


如何追加修改

在本地添加新的路由并从新路由拉取此 PR

git add remote chralpha-master https://github.com/ChrAlpha/Artitalk.git

git fetch chralpha

git checkout -b chralpha-master chralpha/master

同往常一样 commit,从新路由推送回远程分支即可

git push chralpha master

我开启了 Allow edits by maintainers,成员应是有权限追加修改的。


其他

由于每次构建文件过大导致本地 server 读取困难,我连测试 commit 都推送至远程分支了,导致此次 PR commit 较为繁复。如有幸最终能够合并,推荐采用压缩合并方式(squash and merge)将所有 commit 压缩成一条再合并至主分支,防止使主分支过于凌乱。

为不影响现有版本号迭代,所有测试中标记的 Tag 均采用更深一层的版本号(2.4.2 => 2.4.2.x)。如果认为多余也可借助通配符全部删去。

git tag -d 2.4.2.*
git push origin --delete 2.4.2.*

WIP

ChenYFan commented 4 years ago

期待(☆▽☆)

so1ve commented 4 years ago

是的,建议使用babel从class转换, 或者是使用构造函数

Drew233 commented 4 years ago

大佬有空的话可以看一下dev的那个分支,我这几天简单重写了一下(参考着valine),阉割了一些功能(其实是不知道怎么写了芜湖)。 基本上不用var了,因为之前也不是很清楚这些变量声明时候的区别所以全用的var