Skip to content

enhance: resolve error on tencent cdn dupe deployment#273

Merged
usual2970 merged 2 commits intocertimate-go:mainfrom
LeoChen98:enhance-tencent-cdn-dupe-deploy
Oct 29, 2024
Merged

enhance: resolve error on tencent cdn dupe deployment#273
usual2970 merged 2 commits intocertimate-go:mainfrom
LeoChen98:enhance-tencent-cdn-dupe-deploy

Conversation

@LeoChen98
Copy link
Copy Markdown
Contributor

  • 优化:腾讯云cdn重复部署报错的问题

优化:腾讯云cdn重复部署报错的问题
Comment thread internal/deployer/deployer.go Outdated
Copy link
Copy Markdown
Member

@usual2970 usual2970 left a comment

Choose a reason for hiding this comment

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

LGTM

@usual2970 usual2970 merged commit feb851a into certimate-go:main Oct 29, 2024
Comment on lines +153 to +155
if(slices.Contains(deployedDomains, domainStr)){
domains = append(domains, domainStr)
}
Copy link
Copy Markdown
Member

@fudiwei fudiwei Oct 30, 2024

Choose a reason for hiding this comment

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

@LeoChen98 我正在重构 Deployer 的相关逻辑,这个地方没太看懂。从代码上看这里是要取“包含在已部署过的域名里的域名”(即交集)?但从 PR 信息上看似乎是要跳过已部署过的域名才对(即差集)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

草,漏了个 !逻辑反了(x

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

草,漏了个 !逻辑反了(x

😂 没事儿,我在新的 PR 里一并 fix 了吧

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

我改一下吧

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

我改一下吧

🆗

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我改一下吧

辛苦了,我本来想手机改,结果死活登录不上,同步不了fork,有被github app气到

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

我改一下吧

另外,这个问题可能导致腾讯云cdn部署功能不可用,可能需要推紧急发版。私密马赛

usual2970 added a commit that referenced this pull request Nov 5, 2024
enhance: resolve error on tencent cdn dupe deployment
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.

3 participants