Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use testament to check Norm test pass #19018

Merged
merged 4 commits into from
Nov 3, 2021
Merged

Use testament to check Norm test pass #19018

merged 4 commits into from
Nov 3, 2021

Conversation

moigagoo
Copy link
Contributor

This is what I actually use to test Norm, so it's better to use it.

This should not currently pass. This is expected because this is exactly the problem I want to highlight with this PR. My tests do indeed not pass at the moment.

This is what I actually use to test Norm, so it's better to use it.

This should not currently pass. This is expected because this is exactly the problem I want to highlight with this PR. My tests do indeed not pass at the moment.
@moigagoo
Copy link
Contributor Author

Relates to #19017

@ringabout ringabout requested a review from Araq November 3, 2021 00:15
@Araq Araq merged commit b2edc34 into nim-lang:devel Nov 3, 2021
@moigagoo moigagoo deleted the patch-1 branch November 3, 2021 08:29
@moigagoo
Copy link
Contributor Author

It seems like this fix didn't land in 1.6.2. Why is that?

I thought develop is merged into master and a new tag is added during release but apparently this commit from develop wasn't merged.

I was hoping to update Nim version for Norm but I can't since tests won't run on 1.6.0 or 1.6.2.

@narimiran
Copy link
Member

It seems like this fix didn't land in 1.6.2. Why is that?

Because it wasn't marked as [backport].

I thought develop is merged into master

  1. There is no master.
  2. Stuff in devel will become next major release.
  3. Anything that should be backported to next minor release, should be marked as [backport]. E.g. for Nim 1.6.x, commits marked as [backport] are cherry-picked into version-1-6 branch.

@moigagoo
Copy link
Contributor Author

Thanks for the explanation. My bad, didn't know all that, should've learned first

Is it too late to mark it as backport so that it lands in 1.6.4? If there will be 1.6.4.

@narimiran
Copy link
Member

Is it too late to mark it as backport so that it lands in 1.6.4?

It is not too late.

If there will be 1.6.4.

I'm 99% sure there'll be 1.6.4.

narimiran pushed a commit that referenced this pull request Dec 19, 2021
* Use testament to check Norm test pass

This is what I actually use to test Norm, so it's better to use it.

This should not currently pass. This is expected because this is exactly the problem I want to highlight with this PR. My tests do indeed not pass at the moment.

* Remove clearNimblePath from testament command.

Co-authored-by: flywind <[email protected]>
(cherry picked from commit b2edc34)
@narimiran
Copy link
Member

It is backported now, it will be part of the next 1.6.x stable release, whenever that comes out.

@narimiran
Copy link
Member

@moigagoo norm is now red on 1.6.x CIs: https://github.com/nim-lang/Nim/runs/4573499025?check_suite_focus=true (testament: command not found)

@ringabout
Copy link
Member

#19092 needs to be backported too which fixed the other part of the issue.

PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
* Use testament to check Norm test pass

This is what I actually use to test Norm, so it's better to use it.

This should not currently pass. This is expected because this is exactly the problem I want to highlight with this PR. My tests do indeed not pass at the moment.

* Remove clearNimblePath from testament command.

Co-authored-by: flywind <[email protected]>
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.

4 participants