Are you looking for maintainers?

[jasekiw]

I see that there have been many commits in the main branch since the last release back in 2021. I think a new release should be made to include all of these. It appears there hasn't been a lot of activity however. Are you open to adding maintainers to help keep up with the chore work of the library?

Thanks!

[martinnormark]

Hey @jasekiw - thanks for reaching out, and yes I am open to adding maintainers. If you're interested I can add you right away?

I see you're right about a release is pending, I do recall working on some fixes but then everything else took over. I'm not sure if I needed a few fixes included before cutting a new release, but test-wise the library is pretty solid so as they're all passing it should be fine to cut a release.

[jasekiw]

I am definitely interested. This library doesn't seem to be a huge time sink so I can definitely help keep the library updated.

When you say you recall working on some fixes are you perhaps referring to this pr? #348.

I see you have a github action setup for release to nuget so I assume creating a release in github automatically publishes to nuget?

[martinnormark]

Added you as maintainer now. Thanks for reaching out, let me know if you have any questions!

I do recall starting #348 but also #328 seems to be the un-released stuff.

Correct, there's a github action flow that pushes to nuget when tagging a new release here. Only manual thing needed is to update the version and release notes in the project file.

[jasekiw]

I'll start familiarizing myself with the codebase and the unfinished work over the week end. I want to double check everything before making a release and potentially messing something up on my first try. I could look at doing a prerelease be extra careful. I noticed the Draft new release button didn't show on the releases page. I assume you still want to create the release yourself after I let you know a release is ready?

[martinnormark]

It seems that your invite is still pending, did you click "accept" or similar in an email from GtiHub?

[jasekiw]

My apologies, that was it. The repository appeared to show more access but I was mistaken since I hadn't accepted the invite yet. What is your opinion of me doing a prerelease to test waters?

[martinnormark]

Cool, good to hear. Give it a go man, feel at home!

[jasekiw]

I noticed that all of the classes are marked as public. If someone uses the library in a way the readme doesn't state I could accidentally introduce a breaking change for them. This might be too early of a question but how much of library should I assume public api?

[jasekiw]

I can't merge PR's created by me since it requires one approved review but I can't review my own PR. I was able to merge some dependabot PR's however.

[martinnormark]

I can't merge PR's created by me since it requires one approved review but I can't review my own PR. I was able to merge some dependabot PR's however.

Darn, I thought it was possible to avoid reviews for maintainers. I have removed the requirement of a review, since only maintainers can merge PRs it will not be all up for grabs anyway and the review feature is still available.

This might be too early of a question but how much of library should I assume public api?

I see how this could cause a breaking change. I think the main PreMailer class and the interfaces you'd need to customize functionality could be the only public classes. Changing them should be released only in a major version.

[jasekiw]

Thanks for making that adjustment!

I have been going through the issues and closed ones I confirmed are already completed.

I did notice this issue that was not marked as completed but did have a PR that was merged. However I found the next commit reverted it. It seems like an accident.

If there are not reasons against, I'll re-add this functionality to the library but under a configuration flag since some people may want to receive these as errors.

[martinnormark]

I can't recall why that should be reverted, so I agree that it seems like an accident. Would be great to add back in - nice catch, you're on a roll already!

I noticed some possible formatting settings could be out of sync, based on this commit - I thought .editorconfig would prevent that? (entire file being changed)

[jasekiw]

Oh wow, I usually catch large diffs like that. I must have had ignore whitespace checked on my diff. It's probably a line ending issue since it appears on lines with no tabbing too. I'll fix that tomorrow or this week end. Thanks for catching that!

[jasekiw]

@martinnormark

I think I found the root of the issue. It seems that the line endings of the files in the git index are inconsistent. My git configuration is set to checkout as clrf locally and commit as lf which is the default for a windows git installation.

Note: /i stands for index, /w stands for working directory

PS C:\projects\PreMailer.Net\PreMailer.Net> git ls-files --eol
i/lf    w/crlf  attr/                   .editorconfig
i/lf    w/crlf  attr/                   .gitignore
i/lf    w/crlf  attr/                   Benchmarks/Benchmarks.csproj
i/lf    w/crlf  attr/                   Benchmarks/Program.cs
i/crlf  w/crlf  attr/                   PreMailer.Net.Tests/CssAttributeTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/CssElementStyleResolverTests.cs
i/crlf  w/crlf  attr/                   PreMailer.Net.Tests/CssParserTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/CssSelectorParserTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/CssSelectorTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/CssSpecificityTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/CssStyleEquivalenceTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/Extensions/NodeExtensionTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/LinkTagCssSourceTests.cs
i/crlf  w/crlf  attr/                   PreMailer.Net.Tests/PreMailer.Net.Tests.csproj
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/PreMailerTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.Tests/StyleClassApplierTests.cs
i/crlf  w/crlf  attr/                   PreMailer.Net.Tests/StyleClassTests.cs
i/lf    w/crlf  attr/                   PreMailer.Net.sln
i/lf    w/crlf  attr/                   PreMailer.Net/AttributeToCss.cs
i/crlf  w/crlf  attr/                   PreMailer.Net/CssAttribute.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssAttributeCollection.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssElementStyleResolver.cs
i/crlf  w/crlf  attr/                   PreMailer.Net/CssParser.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssSelector.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssSelectorParser.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssSpecificity.cs
i/lf    w/crlf  attr/                   PreMailer.Net/CssStyleEquivalence.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Downloaders/IWebDownloader.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Downloaders/WebDownloader.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Extensions/NodeExtensions.cs
i/lf    w/crlf  attr/                   PreMailer.Net/ICssSelectorParser.cs
i/lf    w/crlf  attr/                   PreMailer.Net/InlineResult.cs
i/crlf  w/crlf  attr/                   PreMailer.Net/PreMailer.Net.csproj
i/-text w/-text attr/                   PreMailer.Net/PreMailer.Net.snk
i/crlf  w/crlf  attr/                   PreMailer.Net/PreMailer.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Sources/DocumentStyleTagCssSource.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Sources/ICssSource.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Sources/LinkTagCssSource.cs
i/lf    w/crlf  attr/                   PreMailer.Net/Sources/StringCssSource.cs
i/crlf  w/crlf  attr/                   PreMailer.Net/StyleClass.cs
i/lf    w/crlf  attr/                   PreMailer.Net/StyleClassApplier.cs
i/lf    w/crlf  attr/                   nuget.bat

This was the Premailer.cs file before my commit:

i/crlf  w/crlf  attr/                   PreMailer.Net/PreMailer.cs

And this is after my commit:

i/lf    w/crlf  attr/                   PreMailer.Net/PreMailer.cs

Since my git installation commits as lf, it converted the file and marked every line as changed.

I recommend that we normalize the line endings in the repository to prevent this issue from creeping up for other contributors as well. I can make a commit using the git command : git add --renormalize . to normalize the git index to lf.
A made a commit here to normalize line endings.

Let me know what you think.

[martinnormark]

Awesome, seems like the right thing to do!

[TopCoder02]

@jasekiw I was excited when I saw this, I was hoping for an update soon. This is a great project.

[jasekiw]

@jasekiw I was excited when I saw this, I was hoping for an update soon. This is a great project.

Hi @TopCoder02 I should have some time this week to try and create a new update. Is there anything specific you were looking for?

[TopCoder02]

@jasekiw Not at the moment, the only thing I think would be useful is to remove the styles that are inherited down and remove the browser defaults from being inlined. I'm doing after I use pre-mailer. I'm probably doing it wrong too.

[jasekiw]

@TopCoder02 A new release has been created see https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0

No browser defaults should be inline from what I am aware of, can you create an issue with a reproduction of what you are referring to?

[jasekiw]

@martinnormark I cut a new release. You can see it here: https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0

I did run into one issue however. The action published the new version but then failed on a subsequent line of code. So it worked but is marked as a failure. https://github.com/milkshakesoftware/PreMailer.Net/actions/runs/6983746731
It appears the action hasn't been maintained in some time and I should probably think about an alternative.

[martinnormark]

@jasekiw Awesome with the new version! You rock!

Those actions for pushing a nuget package all seems to be more or less abandoned, I presume since it is quite easy to do with the dotnet cli these days?

[jasekiw]

@martinnormark That's what I could find from my research so far. I'll play around with publishing a dummy package from a dummy project to make sure things work right and then I'll just copy the cli command over into the action.

[TopCoder02]

@TopCoder02 A new release has been created see https://github.com/milkshakesoftware/PreMailer.Net/releases/tag/v2.5.0

No browser defaults should be inline from what I am aware of, can you create an issue with a reproduction of what you are referring to?

I was referring to https://www.w3schools.com/cssref/css_default_values.php

If someone has the style

<style>
h1 {
display: block;
font-size: 2em;
margin-top: 0.67em;
margin-bottom: 0.67em;
margin-left: 0;
margin-right: 0;
font-weight: bold;
}
</style>

Since these values are already the default value, it shouldn't be inlined on an h1 tag, even if it's part of a CSS Class. Since it's already defaulted. I know it's more complicated than this, but that's what I was thinking, it will make the CSS smaller and email size smaller. Because it's more intelligent in inlining, removing unnecessary styles.