Skip to content

Replace DotLiquid with Fluid#3679

Merged
RicoSuter merged 1 commit intoRicoSuter:masterfrom
lahma:fluid
Dec 2, 2021
Merged

Replace DotLiquid with Fluid#3679
RicoSuter merged 1 commit intoRicoSuter:masterfrom
lahma:fluid

Conversation

@lahma
Copy link
Copy Markdown
Collaborator

@lahma lahma commented Oct 26, 2021

Still work in progress, uses preview feed for NJsonSchema.

  • update templates to follow strict liquid syntax
  • net461 minimum full framework
  • netstandard 1.3 removed, netstandard 2.0 is minimum when generating code using templates

@sebastienros
Copy link
Copy Markdown

FYI EscapeDataString has been removed in 6.0-rc2. I see it's used in the resulting code.

@RicoSuter
Copy link
Copy Markdown
Owner

@sebastienros and what's the replacement for it?

@lahma
Copy link
Copy Markdown
Collaborator Author

lahma commented Oct 29, 2021

@RicoSuter thanks for helping out here, how's it looking from your perspective? Seems that the libs in VCS didn't restore correctly on ADO build

@RicoSuter
Copy link
Copy Markdown
Owner

RicoSuter commented Oct 29, 2021

There are some line breaks which we should remove:

NJS:
image

NSwag:
image

Couldnt find where these are yet...
Otherwise it looks good so far...

Seems that the libs in VCS didn't restore correctly on ADO build

Yes, the build pipeline needs to be improved so that we only have one (ideally ADO) - currently we have two: Appvayor (Full) and ADO (partial, couldnt get full to run there)... Hopefully the nuke build improves/simplifies that... Also a problem for that is that there is a lot of legacy support (full .net etc.).

@lahma
Copy link
Copy Markdown
Collaborator Author

lahma commented Oct 29, 2021

I think I can check these these extra line breaks this weekend the latest unless you find out something before.

I would be a bit cautious with ADO though, MS has shifted interest to GH actions (I'd even argue they are sunsetting ADO) and they are far faster than ADO by what I have observed. GH actions also integrate a lot better with GH PR status UI.

Let me know if you would like me to add GH actions workflows to NUKE PR.

@sebastienros
Copy link
Copy Markdown

Actually it might be another one that is obsoleted: EscapeUriString dotnet/runtime#41501

@RicoSuter
Copy link
Copy Markdown
Owner

Let me know if you would like me to add GH actions workflows to NUKE PR.

It makes sense to improve the CI and GH actions are fine for me.
We should ensure that you can run the full build easily locally and the CI build should produce all artifacts...
What about the CD pipeline? ie pushing the nuget and npm packages and releasing the NSwagStudio MSI (maybe we use a regular GitHub release for that)?

@sebastienros
Copy link
Copy Markdown

sebastienros commented Nov 10, 2021

There are nupkg files in the PR

@lahma
Copy link
Copy Markdown
Collaborator Author

lahma commented Nov 11, 2021

It makes sense to improve the CI and GH actions are fine for me. We should ensure that you can run the full build easily locally and the CI build should produce all artifacts... What about the CD pipeline? ie pushing the nuget and npm packages and releasing the NSwagStudio MSI (maybe we use a regular GitHub release for that)?

We can do these, but can you describe how it currently works, is AppVeyor doing something all is everything manual? I saw there's 04_Publish.bat which only does NPM publish - is it used in automation or your personal tool?

I think NUKE could do following steps in Publish target:

  • triggered by new GH actions decription for tag happening with prefix v on master, e.g. v14.5.2
  • if there's a tag and we are on main/master - continue, otherwise skip publish
  • build, test, pack
  • create GitHub release with name taken from the tag, e.g. 14.5.2 via github API (requires API key)
  • upload .msi and .zip to new release
  • do dotnet nuget push to nuget.org (requires NuGet API key)
  • push nswagstudio nupkg to chocolatey - I've never done this, requires Yet Another API Key?
  • do NPM publish (requires NPM API key)

So I could make base setup for you as a PR, but you basically are only one to complete the challenge with access to keys etc. Probably will make sense to do "beta" releases.

@sebastienros
Copy link
Copy Markdown

I think NUKE could do ...

Nothing too

@sebastienros
Copy link
Copy Markdown

I just lost a valuable contributor ;)

@lahma lahma force-pushed the fluid branch 4 times, most recently from 6cbc273 to bd7e4eb Compare November 17, 2021 09:11
@lahma lahma force-pushed the fluid branch 2 times, most recently from debb5a0 to 020b0a7 Compare November 25, 2021 15:42
@lahma lahma marked this pull request as ready for review November 25, 2021 15:58
@lahma
Copy link
Copy Markdown
Collaborator Author

lahma commented Nov 25, 2021

What a emotional moment, build passes, tests are green.

@lahma lahma force-pushed the fluid branch 2 times, most recently from dd3ff53 to ffb5359 Compare December 2, 2021 20:03
* update templates to follow strict liquid syntax
* net461 minimum full framework, remove netstandard1.3
@RicoSuter RicoSuter merged commit 87d7ca7 into RicoSuter:master Dec 2, 2021
@lahma lahma deleted the fluid branch December 2, 2021 20:37
lahma added a commit to lahma/NSwag that referenced this pull request Dec 25, 2022
* update templates to follow strict liquid syntax
* net461 minimum full framework, remove netstandard1.3
lahma added a commit to lahma/NSwag that referenced this pull request Jan 20, 2024
* update templates to follow strict liquid syntax
* net461 minimum full framework, remove netstandard1.3
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