Avoid golint failures: BlockHtml

[sayanarijit]

blackfriday/html.go

Line 241 in f1f45ab

func (options *Html) BlockHtml(out *bytes.Buffer, text []byte) {

Hi, I think we should rename BlockHtml it to BlockHTML and create new function BlockHtml pointing to BlockHTML to avoid golint failures maintaining compatibility.

[sayanarijit]

Also

blackfriday/markdown.go

Line 186 in f1f45ab

RawHtmlTag(out *bytes.Buffer, tag []byte)

[crazcalm]

Do you guys mind if I pick this ticket up?

To be clear, you want BlockHtml and RawHtmlTag to be renamed to BlockHTML and RawHTMLTag, and then create the methods BlockHtml and RawHtmlTag that point to the renamed functions, correct?

[sayanarijit]

@crazcalm I think this is more complicated than this. Actually I hoped for more comments on this from the maintainers.

Here are my points:

• It's not only BlockHtml and RawHtmlTag. There are many other declarations like HtmlRenderer causing golint failures in this repo and external repos using blackfriday API (e.g. kubernetes issue 68026). In order to solve this we need to go file by file checking for golint failure cases.

• Renaming and linking functions will help users of this library resolve golint failures in their repo. but it won't resolve golint failures in this repo itself. Moreover there are structs using name like HtmlRendererParameters which can not be resolved so easily without breaking compatibility.

• If we are planning for V2 as tagged by @rtfb I don't think we need to worry about compatibility part as users should expect breaking changes in major version release. But if we merge v2 with master, user codes using weak dependency management tools will break. We can adopt Semantic Import Versioning and release V2 in a sub folder like github.com/russross/blackfriday/v2 to take care of this.

To sum it up, I opened this ticket hoping to resolve a part of golint failure issue in Kubernetes but after doing some deep dive of this library, found it's much more complicated than this.

[rtfb]

Yeah, there was a bit of jerk reaction on my side... First, I misapplied the v2 tag. The issue is about public names in master (i.e. v1), and I thought that would be something in v2. And second, I didn't pay attention that even if it's v2, it's already released and we can't change the public names any more. So if we still have some lint issues in v2, we'll have to live with them for quite a while.

So there's really no way to completely resolve this issue -- the linter-offending names must stay. The best we can do is to introduce the fixed names, and mark the old ones as deprecated.

Having said that, I don't think renaming things in v1 is worth the effort at all. A large part of motivation for v2 was cleaning up the names of exported identifiers. So the right way to avoid linting issues is to migrate to v2. (But I suspect that v2 is not completely free of lint issues either, because I wasn't using a linter while working on it...)

If we are planning for V2 as tagged by @rtfb I don't think we need to worry about compatibility part as users should expect breaking changes in major version release. But if we merge v2 with master [...]

v2 is already released, so we have to care about compatibility there as well. What public identifiers were not fixed, will have to stay until v3. But we're not going to merge with master. The plan prior to widespread dependency management tool usage was to keep master as v1, and have v2 branch as "master" of v2. So far it seems like we should stick to this plan.

[sayanarijit]

@rtfb thanks for the explanation. Now I think it's clear to me. So as I understand, we will be creating/merging PRs to the V2 branch and not master. And for my purpose of fixing golint failures in Kubernetes, I need to check which version is being used there and if it's V1, make proposal to migration to V2.

[sayanarijit]

Or if we are not going to change V2, shall we close this issue? As I don't see a V3 branch as of now, I guess V3 will be all linted when it will be released.