Skip to content
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

記事更新時に更新元ファイルのentryHeaderのURLが未設定の場合も更新ができるように修正 #99

Merged
merged 2 commits into from
Oct 14, 2023

Conversation

halkt
Copy link
Contributor

@halkt halkt commented Oct 14, 2023

背景

解決策の提案

  • PutEntryの処理内では, 更新元ファイルにURLが未設定の場合はorigPathに空白を返すように変更しました
    • この変更が適切かどうかやや心配なところもありますので, 一度ご確認いただけますと幸いです

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6518699332

  • 5 of 6 (83.33%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+3.7%) to 30.636%

Changes Missing Coverage Covered Lines Changed/Added Lines %
broker.go 5 6 83.33%
Totals Coverage Status
Change from base Build 6518420872: 3.7%
Covered Lines: 212
Relevant Lines: 692

💛 - Coveralls

@Songmu
Copy link
Member

Songmu commented Oct 14, 2023

なるほど。こっちで壊してしまいましたか。すみません。workflowが壊れてしまうのは困るので、これは一旦受け入れます。

ちなみに、URLを削っている理由は、下書き段階でcustom-pathが指定されていない場合にURLがコロコロ変わってしまうからでしょうか?

@Songmu Songmu merged commit 3c914cc into x-motemen:master Oct 14, 2023
7 checks passed
@github-actions github-actions bot mentioned this pull request Oct 14, 2023
@halkt
Copy link
Contributor Author

halkt commented Oct 14, 2023

早速ありがとうございます!

URLを削除しているのは,おっしゃる通りcustom path の指定がない場合にURLが変わってしまうことと公開時にURLを確定させるはてなブログの思想に合わせた形になります

ちなみにDATEについても同じ理由で下書き状態の場合は削除するようにしています

@Songmu
Copy link
Member

Songmu commented Oct 15, 2023

そうですよね。Dateに関しては以下でDraft時にはセットしないように変更していました。URLに関してもそうしてもいいかもですね。
#95

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