Skip to content

Conversation

@demarey
Copy link

@demarey demarey commented Sep 27, 2023

No description provided.

@noha
Copy link
Owner

noha commented Oct 4, 2023

There are some things I don't like in this PR.

You changed the header format which is not ok. The header is supposed to be MIME compatible with STON as value format. Introducing separated values by ; is not good not yaml if you ask me.

I don't like it either that some inferior format like yaml sneaks in in a central class where there is if/else switches all over the place. It is kind of decay of software just like adding dependencies.

Finally you guys (researches maybe 😉) need to stop consider code that you don't use as unused. I use the WebSite build method. It was the first one introduced before esteban introduce a builder for his purpose. So what you need to say is that there might be a test missing but just removing code needs a bit more insight as this.

Sorry! I like the tests but the direction this PR adds is not a really good to me. Maybe we should discuss this because I cannot see the use of the format you call yaml.

@demarey
Copy link
Author

demarey commented Oct 6, 2023

Hi Norbert,

I did not change the header format. I just add the support for a new header format that is often used in markdown. I did not change anything to parsing of values. If this new header format is a problem, I can easily change it to what is used in microdown: { ... } that is directly parsable by Ston, also enforcing to have opening and closing brackets followed/preceded by a new line.

there is if/else switches all over the place.

Where do you see that? There is one single if to determine if it is the legacy header (with no delimiter) or the yaml metadata header. Also, I now really find the metadata parser more readable.
We should just discuss what we would like to keep:

  • legacy foliage metadata header: I do not like it because there is no separator and we do not know what we parse.
  • yaml metadate header: full header or we just keep the separators
  • Microdown metadata header: maybe the best option
{
    "title": "Selecting Expressions",
    "author": "S. Ducasse and L. Fabresse with Q. Ducasse",
    "subtitle": "",
    "slidesid": "W1-LiveA-EN"
}

Finally you guys (researches maybe 😉) need to stop consider code that you don't use as unused.

Why do you extrapolate? It is just me that found no usage of a method. If it is used, let's keep it.

I'm available to discuss. Ping me when you are available.

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