Cocoanetics / DTMarkdownParser

An event-based parser for markdown text
BSD 2-Clause "Simplified" License
197 stars 15 forks source link

Jan/range array more #12

Closed JanX2 closed 10 years ago

JanX2 commented 10 years ago
coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 7b4a59058b39a7f2d3f05b5236483eb51bf2eaf1 on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

odrobnik commented 10 years ago

Before I can merge this:

You can avoid the -description methods from being counted (no sense in unit testing those) by adding

#ifndef COVERAGE
// not visible to coverage
#endif

Or you can remove the unused methods since this is an internal class not part of the public API.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 80d10c25849ea3a2ee54c541ef11319127190c5a on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling c135e6fc26ff86ce13eaa16db3808dec10ffc3bf on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

odrobnik commented 10 years ago

here's the latest coverage report, still some work remaining. https://coveralls.io/files/77766412

odrobnik commented 10 years ago

_lineIndexContainingIndex: does not seem to be used any more, you should remove it.

JanX2 commented 10 years ago

implementing a unit test for description is relatively

Did that just for the fun of it. :)

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 2524743f5c957afaf56a1b1365887f0709064ccb on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 589606b4db7b7bc62a845cbbb387819684d6d5be on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 9bf13382f7bfed1cf1f3166dc2832d7b6f890a67 on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

JanX2 commented 10 years ago

Please keep/modify the class prefix generally DT - to avoid potential conflicts if somebody is using JXRangeArray in some other project, otherwise you risk duplicate symbols.

I would prefer to:

As this is OSS, anyone needing a different version of JXRangeArray later on can just fork this repo and update the submodule then (or just modify JXRangeArray in-place).

As far as I understand somebody using DTMarkdownParser doesn't have to give attribution, right? Just keep the copyright in the header in tact, yes?

That is my understanding as well when considering source code, as the license is included. Binary builds will have to include the license text as per the license.

Please make sure that Code Coverage for your added classes is 100%. Right now the class is only at 76. I am getting close!

You can avoid the -description methods from being counted…

Done.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 3372ee8d4b4a0f32a687edc30ba602cb0aa79cfa on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

odrobnik commented 10 years ago

Sorry, but i have to insist on renaming the class.

JanX2 commented 10 years ago

Why? Are there any more technical reasons for doing so?

odrobnik commented 10 years ago

If the code is local then it would be causing a conflict when being used together with another project using the same class. It is not ready to be a submodule (missing docs, no cocoapod, no repo) and adding a dependency for only this is inconvenient for users of this. I know painfully from other projects that people often don't understand how to also get the dependencies.

I see no other way.

JanX2 commented 10 years ago

If dependencies ever are a problem, you can use fake submodules. They are a bit of a bitch when it comes to making changes locally, though. You have two histories to maintain. The one in your repo and the other in the fake submodule.

Anyway. Let’s cross the renaming-issue-bridge once I actually move JXRangeArray to it’s own repository. It’s pretty improbable someone using it will cause an accidental collision before that.

We can then include it via a fake submodule and use a branch with commits that do all the renaming for internal use here. If there are upstream changes in JXRangeArray, it’s easy to rebase the renaming on the upstream changes. Been there, done that. It’s easy and it’s also easy to maintain. Better than a manual rename in any case.

I promise to do the renaming once JXRangeArray does everything we need here, at which point I will move it to it’s very own repo.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling b4d76d90ef6a73e32076728aad49400211fa6943 on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

odrobnik commented 10 years ago

You are missing the point: I have decided that there will be no submodule fake or otherwise.

Rename the class or I can never merge it in.

JanX2 commented 10 years ago

Fake submodules evade the problem you have described: people often don't understand dependencies. Those people will never have any problem as the files are all right there in the main repo.

I do not want to rename the class now only to have to rename it back later. That would be silly.

JanX2 commented 10 years ago

In any case: OCMockito already is a submodule.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling a2d5dbdeecf4c54822e4828aaf8aa8b28d706b50 on JanX2:jan/range-array-more into 47fb86ba0fa82eae3cd4d846e82ee0f956ac2cdb on Cocoanetics:develop.

odrobnik commented 10 years ago

I have removed the OCMockito dependency a long time ago.

There is no point arguing with me over my decision and goal related to submodules, fake or otherwise. I am not going to merge this ever.