Add the Node that comes with Visual Studio as a NodePath fallback in the .targets file in the Microsoft.TypeScript.MSBuild Nuget package

I am working on a new project using ASPNET Core 3.1 and I wanted to write our font-end code in TypeScript, but found a little problem: You can't use TypeScript and build the project using the dotnet CLI commands if you don't have a stand alone installation of Node. If you build using Visual Studio, it works ok because VS now comes with Node, but building in Azure Pipelines or local with the "dotnet build" command results in an error saying Node could not be found.
My project is referencing the Microsoft.TypeScript.MSBuild nuget package.
On March 20th 2020, I created a simple repro at https://github.com/kelps/TypeScriptBuildRepro and also submitted a VS feedback about this at https://developercommunity.visualstudio.com/content/problem/957892/typescript-files-do-not-build-using-dotnet-cli-com.html.
@timheuer helped me diagnose it and the problem was that Node needed to be in the Path to work. I didn't like that because Visual Studio already comes with Node and I shouldn't need to have another separate Node install to run this. @madskristensen's BuildWebCompiler Nuget package also uses Node to compile .less/.sass/... files and it just works, without a stand alone Node install.
After a long time testing and researching I was able to create a very simple work around. I added a msbuild Task to my .csproj file that runs before the TypeScript compilation and sets the NodePath to the one that comes with VS if it is still empty. When it worked, I removed that code from my .csproj file and placed it in the Directory.Build.targets file in the solution directory.
My suggestion is to add some form of that code to the .targets file in the Microsoft.TypeScript.MSBuild Nuget package.
The master in my repo now has the code that works. To see the build failling, just rename the Directory.Build.targets file and try to build the solution with "dotnet build" in a computer without a stand alone Node installation (or at least without Node in the %path%).
It is a simple "fix" that provides a good fallback for TypeScript compilation with ASPNET Core on Windows. I am sure my code can be improved, but it is a good start. It uses vswhere to find the VS Node path.
For reference, here is the code in my .targets file.
<Project ToolsVersion="15.0">
  <!--https://stackoverflow.com/questions/31664834/customize-system-environment-variable-path-for-msbuild-exec-task#31670922-->
  <!--http://blog.jdhardy.ca/2011/12/setting-environment-variables-for.html-->
  <!--bat script for findind the Visual Studio instalation path for Node and setting it in the appropriate variable before building TypeScript files-->
  <PropertyGroup>
    <SetNodePath>
    <![CDATA[
	@setlocal enabledelayedexpansion
	@for /f "usebackq tokens=*" %%i in (`"%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere" -latest -find **\node.exe`) do (@set nodePath=%%~dpi)
	@echo %NodePath%
    ]]></SetNodePath>
  </PropertyGroup>

  <!--Target that runs the script above if the node path isn't set yet and it is running on Windows-->
  <Target Name="FindAndSetNodePath" BeforeTargets="PreComputeCompileTypeScriptWithTSConfig" Condition="'$(NodePath)' == '' AND '$(OS)' == 'Windows_NT' AND exists('$(MSBuildProgramFiles32)\Microsoft Visual Studio\Installer\vswhere.exe')">
    <Exec Command="$(SetNodePath)" ConsoleToMSBuild="true">
      <Output TaskParameter="ConsoleOutput" PropertyName="NodePath"/>
    </Exec>
    <Message ContinueOnError="true" Text="NodePath: $(NodePath)" Importance="high" />
  </Target>
</Project>

Thanks @kelps , while I think your workaround is brilliant, I'd like to be very careful about making this a part of the TypeScript package. The reason being that in general we want our build tools to be as deterministic as possible, and using vswhere (or an equivalent) will make the package even more environment-dependent. This is something we generally want to avoid -- the obvious reason being the one you've already called out: it might work in your local environment, but not work when you deploy it to something like a CI machine. Or your build may subtly change without you noticing when you install different versions of VS side by side, or you uninstall VS. This solution may solve this specific scenario for you, but I'm not convinced it applies to most users.
While using %PATH% is also obviously very environment-dependent, the alternatives where "build doesn't work outside VS at all" and "bundle Node.exe in the Nuget package" and we found this an acceptable option.

I do understand your point, but I still think this is a good fallback to put in the package, because it already has other 2 similar fallbacks in the .targets file that point to other (maybe legacy) paths. I was only able to make my workaround succeed because I saw what the package's .targets file was already doing.
As I see it, this will only increase the cases where the Nuget package just works (becoming a little more deterministic). It already has a check if vswhere exists and only runs if that is true and the NodePath is still empty, so, in a case where vswhere isn't available, the build will simply behave the same way it does today. But when we do have vswhere and Node isn't in the Path, it gives an extra chance for the build to work.
I do hope this improvement (or something similar) would be considered.
Thanks

Note: this does not address the original problem, but more offers a (probably undesirable) workaround:
We use a  .njsproj for our front-end development and it builds just fine on Azure Pipelines when using the Visual Studio Build command (presumably because it's using JSTools and not a nuget package?). Do you have a separate project for your front end, or is it all in the same .net project as the backend?
For reference, we're also using BuildWebCompiler with no issues.

Because I'm bad at checking github for issue updates, and in case you decide to try using a .njsproj here are the instructions for installing the BuildWebCompiler nuget package into a .njsproj:
Add the following lines to the end of your .njsproj file:
  <Import Project=".\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets" Condition="Exists('.\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets')" />
  <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
    <PropertyGroup>
      <ErrorText>This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
    </PropertyGroup>
    <Error Condition="!Exists('.\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets'))" />
  </Target>

(replace the version number with the version of BuildWebCompiler you actually want)
Add a packages.config file to the directory of your project with the following:
<?xml version="1.0" encoding="utf-8"?>
<packages>
  <package id="BuildWebCompiler" version="1.12.394" targetFramework="net40" />
</packages>

(again change the version number as appropriate)
On Azure Pipeline add a NuGet restore command version 2.* with the path set to your project's packages.config (e.g. Client\packages.config).
On developer machines you will need to install it manually because .njsproj doesn't support .nuget packages by default. I've committed a script file to my repo that does the following:

Download nuget.exe from https://www.nuget.org/downloads.
Open command prompt and navigate to your project's directory and enter: "nuget.exe" restore -PackagesDirectory "./packages".

I hope any of this is helpful!

Note: this does not address the original problem, but more offers a (probably undesirable) workaround:
We use a  .njsproj for our front-end development and it builds just fine on Azure Pipelines when using the Visual Studio Build command (presumably because it's using JSTools and not a nuget package?). Do you have a separate project for your front end, or is it all in the same .net project as the backend?
For reference, we're also using BuildWebCompiler with no issues.

Because I'm bad at checking github for issue updates, and in case you decide to try using a .njsproj here are the instructions for installing the BuildWebCompiler nuget package into a .njsproj:
Add the following lines to the end of your .njsproj file:
  <Import Project=".\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets" Condition="Exists('.\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets')" />
  <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
    <PropertyGroup>
      <ErrorText>This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them.  For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}.</ErrorText>
    </PropertyGroup>
    <Error Condition="!Exists('.\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets')" Text="$([System.String]::Format('$(ErrorText)', '..\packages\BuildWebCompiler.1.12.394\build\BuildWebCompiler.targets'))" />
  </Target>

(replace the version number with the version of BuildWebCompiler you actually want)
Add a packages.config file to the directory of your project with the following:
<?xml version="1.0" encoding="utf-8"?>
<packages>
  <package id="BuildWebCompiler" version="1.12.394" targetFramework="net40" />
</packages>

(again change the version number as appropriate)
On Azure Pipeline add a NuGet restore command version 2.* with the path set to your project's packages.config (e.g. Client\packages.config).
On developer machines you will need to install it manually because .njsproj doesn't support .nuget packages by default. I've committed a script file to my repo that does the following:

Download nuget.exe from https://www.nuget.org/downloads.
Open command prompt and navigate to your project's directory and enter: "nuget.exe" restore -PackagesDirectory "./packages".

I hope any of this is helpful!


RSpec/Rails/AvoidSetupHook consider minitest setup hook as problem

Hi there, I got rails project with a bunch of legacy code, and for some forgotten reason it uses both rspec and minitest tests
If I run rubocop with auto-fix in root folder - it will consider this as a problem and fix it and broke minitest file
rubocop test/controllers/clients_controller_test.rb
Inspecting 1 file
C

Offenses:

test/controllers/clients_controller_test.rb:6:3: C: [Correctable] RSpec/Rails/AvoidSetupHook: Use before instead of setup.
  setup do ...
  ^^^^^^^^

1 file inspected, 1 offense detected, 1 offense auto-correctable

And after applying fix I got:
rake test
rake aborted!
NoMethodError: undefined method `before' for ClientsControllerTest:Class

I think this cop should ignore files containg minitest tests, maybe by detecting class ClientsControllerTest < ActionController::TestCase or something like this

Just exclude your tests from rubocop-rspec's attention focus, see the documentation on how to achieve that.
There's rubocop-minitest if you need to check the style of your Minitest tests.

I think there is something wrong in documentation

By default, rubocop-rspec only inspects code within paths ending in _spec.rb or including spec/

My file is not either of this and named test/controllers/clients_controller_test.rb
And there is no other Include in my config which looks like this

@ShockwaveNN Ha! The documentation is correct and corresponds to the default configuration. I can't remember seeing rubocop-rspec inspecting outside */spec/, and if it does, it's either a misconfiguration, or a regression in rubocop.
I can reproduce the issue with the repository you provided with RuboCop 1.16.1, though. Can't see anything suspicious in your configuration.
The problem seems to be somewhere here:
# rubocop-1.16.1/lib/rubocop/cop/team.rb:83
on_duty = roundup_relevant_cops(processed_source.file_path)

and deeper inside:
# rubocop-1.16.1/lib/rubocop/cop/team.rb:158
def roundup_relevant_cops(filename)
  cops.reject do |cop|
    cop.excluded_file?(filename)

# rubocop-1.16.1/lib/rubocop/cop/base.rb:229:
def excluded_file?(file)
  !relevant_file?(file)
end

# rubocop-1.16.1/lib/rubocop/cop/base.rb:223
def relevant_file?(file)
  file == RuboCop::AST::ProcessedSource::STRING_SOURCE_NAME ||
    file_name_matches_any?(file, 'Include', true) &&
      !file_name_matches_any?(file, 'Exclude', false)
end

> on_duty.first.name
=> "RSpec/Rails/AvoidSetupHook"
> on_duty.first.relevant_file?(processed_source.file_path)
=> true
> on_duty.first.__send__(:file_name_matches_any?, processed_source.file_path, 'Include', true)
=> true
> processed_source.file_path
=> "/home/pirj/source/tmp/testing-wrata/Gemfile"

@koic @bbatsov Can you please transfer this issue to rubocop?
Wondering if there were some changes made that could affect how Include merging between cops's own config and AllCops/Include? As far as I remember, cops' and extensions' configs should override the default broad list of Includes according to https://docs.rubocop.org/rubocop/1.9/configuration.html#unusual-files-that-would-not-be-included-by-default.

@pirj Thanks for your investigation
I can simplify my repo to minimal reproducible case if this needed by @koic or @bbatsov, because right now project is rather complicated

That would be helpful, thanks!

@pirj actually actual case is rather simple case for new empty rails app
gem install rails # <IP_ADDRESS>
rails new rubocop-rspec-issue-1160 --skip-webpack-install
cd rubocop-rspec-issue-1160
bundle add rubocop-rspec
rails generate controller Articles

cat <<EOT > .rubocop.yml
require:
  - rubocop-rspec

AllCops:
  NewCops: enable
EOT

cat <<EOT > test/controllers/articles_controller_test.rb
require "test_helper"

class ArticlesControllerTest < ActionDispatch::IntegrationTest
  setup do
    @a = 'b'
  end
end
EOT
rake test # no error, 0 tests
rubocop -A test/controllers/articles_controller_test.rb
rake test # NoMethodError: undefined method `before' for ArticlesControllerTest:Class

I've attached this project as archive
rubocop-rspec-issue-1160.tar.gz
rubocop -V
1.16.1 (using Parser <IP_ADDRESS>, rubocop-ast 1.7.0, running on ruby 2.7.3 x86_64-linux)

TIL bundle add ...

Pinging @koic @bbatsov to receive some info about this issue

@koic @bbatsov @marcandre @jonas054 Can you please transfer this issue to the mainland RuboCop repo?
I can take a look in a while if you happen not to spot the problem right away.

I don't see a UI element to do this.

@marcandre I think you should be admin of organization or have right access to both source and destanation repo

This problem is very similar to rubocop/rubocop#9889 and we were able to resolve that problem, but I failed to realize that the solution should not be in @dmke's local configuration but in the configuration of rubocop-rspec.
The crux of the matter is that configuration for the RSpec department isn't automatically inherited down to subdepartments like RSpec/Rails. I'm not sure if we could change RuboCop so that kind of inheritance takes place. The other solution would be to add
RSpec/Rails:
  Include:
    - "**/*_spec.rb"
    - "**/spec/**/*"

in config/default.yml of rubocop-rspec.

@jonas054 Ah, that is now clear. Sure, that's an easy option, and it's justifiable.
Do you think there are other extensions that have sub-departments apart from rubocop-rspec?
@ShockwaveNN Would you like to contribute such a change? Appreciate if you also fix it for other departments, RSpec/Capybara and RSpec/FactoryBot.

@pirj Actually I'm not very familar with Rubocop internal mechanics and config default format, I usually only report bugs )
So I'm little bit worried that I may broke something and not sure I'll be able to write a correct test suite

I forgot to mention it earlier (for a half-a-year), sorry
But I can confirm that this issue is fixed on current stable rubocop-rspec


Q: How to use bootstrap-sass with @extend and mixins

I tried to ask this on Stackoverflow, but did not get any response yet. My question:
I'm trying to use Twitter Bootstrap with Angular 2, SCSS and the scaffold generated by angular-cli
I want to apply the best practice to not litter my markup with Bootstrap-specific CSS classes. Instead I want to apply/use them via @extend or as mixins:
.mybutton {
   @extend .btn;
}

My problem:

The SASS compiler throws an error, because it does not know about  .btn
If I add @import "bootstrap"; to my .scss-File, it will render the full Bootstrap-CSS in every component css

Any clues for me how this can be matched with the Angular 2 / Angular-CLI approach of handling styles?
What I've done so far

add @import "bootstrap"; to my app.component.scss

Apply ViewEncapsulation.None to the AppComponent, so Angular 2 does not apply Shadow DOM emulation and the Bootstrap CSS applies to the whole application
 @Component({
    encapsulation: ViewEncapsulation.None
 })

Include the following block into angular-cli-build.js, so relative @import will work
sassCompiler: {
  cacheExclude: [/\/_[^\/]+$/],
  includePaths: [ 'node_modules/bootstrap-sass/assets/stylesheets' ]
},
vendorNpmFiles: [
    ...
    'bootstrap-sass/assets/**/*'
]

Did you find any solution for that?

Unfortunately not. Currently I do litter my HTML code with foreign Bootstrap classes. Seems to be a nice problem I have here.

Tbh I'm not really sure how to reconcile this with the modular approach that encourages shadow DOM emulation.

Anybody find a solution?

Yes, you can do it like explained in the aswer at stackoverflow:
http://stackoverflow.com/questions/38071790/angular2-how-to-use-bootstrap-sass-with-extend-and-mixins-in-angular-cli

why not just import bootstrap into the root styles.scss file that is inlined on the index page? then you don't need to tweak encapsulation

I Tryed all this things... but he still dont find the mixins in my IDE (PhpStorm) Compiling is no problem. but it sucks that i dont got autocomplete in PhpStorm...

Looks like you have an answer, also the documentation for bootstrap is improving to include bootstrap-sass with this PR: #4115

The problem isn't importing bootstrap into an app; the problem is described by the OP:

If I add @import "bootstrap"; to my .scss-File, it will render the full Bootstrap-CSS in every component css.

This means if you don't want a new copy of bootstrap included with every component you need its variables/mixins for, you need to define bootstrap-dependent parts of your component styles in the global stylesheet.
Ideally, the bootstrap SCSS would be imported once globally, and then its variables/mixins would be available to any component without having to re-import the whole thing.

@spongessuck You nailed it. I always wondered what's so special about my wish to keep my code clean from Bootstrap classes that nobody else stumbles over it. Seems that most people are happy with directly using and assign Bootstrap. I would like rather to follow "Please stop embedding Bootstrap classes in your HTML!"

@bentolor I have no problem with using BS classes in my templates, and that works fine when it's included globally. I'm more interested in being able to @extend bootstrap classes and use bootstrap breakpoint mixins in my component stylesheets, but I think the solution to both problems is the same.

@bentolor @spongessuck We can import e.g. bootstrap-grid.scss in styles.scss (root) and use the mixins in this root file only. By using /deep/ we can make classes available to child modules/components. However this is no desirable approach because we then lose the ability to separate our css classes by components (at least for those classes that need mixins).
Another approach is to import only the mixins e.g.:
@import '../../node_modules/bootstrap/scss/_variables.scss';
@import '../../node_modules/bootstrap/scss/_mixins.scss';

:host {
  @include make-row();
}

This way you only have to include the grid in your root scss file.
Do you guys have any other updates on this issue?

Do you guys have any other updates on this issue?

@ArtworkAD Not sure what you mean; having to re-import these bootstrap pieces over and over is still a problem.

@spongessuck I mean that by importing just the mixins you will not have multiple copies of bootstrap css.

@ArtworkAD obviously it's better than importing the whole thing, but there's still duplication.
What would have to be smarter in order to only include imported styles once- Sass or the Webpack config? Something else? Do imports in component stylesheets currently get processed for view encapsulation?

@Brocco why did you close this issue?  It is a real problem that should be reopened and fixed.
Having an ugly work around that is not maintainable is not a solution.

I dont believe this issue that is so obvious has not been solved yet !! all devs who used this starter never heard of Rule#1 in development Don't Repeat Yourself !! How on earth were you including General Sass mixins and variables in every component ?!

guys? anybody? A little hint in angular-cli would also help.

I currently moved on to VueJS and never found a good solution with Angular CLI.
AFAIK the current Angular CLI now also uses Webpack2 which can make a very good job in removing unnecessary duplicates in the bundled resources. VueJS i.e. uses by default unscoped CSS while Angular 2 uses scoped CSS. The later requires duplication of all declarations as they are limited to the scope.
Honestly – I could live with repeating imports if it was not duplicating all the imported CSS for every component as well.

Any news on this?
I would love to use bootstrap mixins while using the CSS encapsulation.

@Nkmol me too

@bentolor Don't you find the same problem with VueJS? I'm using vue components and I'm having the same issue.
I include the bootstrap sass file in main.js, which allows components to use bootstrap CSS but not any bootstrap mixins.

I have a couple of options that might work for folks who would like to utilize bootstraps mixins for more semantic markup.
Please note that both options are assuming you have installed
npm install<EMAIL_ADDRESS>Option 1:
Don't use the style urls in your decorator and import your components styles into styles.scss
I usually have a partial named _base.scss that imports my global and vendor styles for example:
@import "variables";  @import "mixins";  @import "bootstrap";
Then in your styles.scss you would import the base partial and then your components
`@import "base";
// add your components here
@import "../app/nav-bar/nav-bar.component";`
Option 2:
Use the style urls in your decorator and Import each dependancy into your component for example importing the following into your component will allow you to use the grid mixins for more semantic markup:
my-component.scss
@import "../../../node_modules/bootstrap/scss/variables";
@import "../../../node_modules/bootstrap/scss/mixins";
@import "../../../node_modules/bootstrap/scss/grid";`
Keep in mind when using option 1 you have to manually add your components styles to your main style sheet. And with option 2 you have to import bootstraps variables, mixins and grid into each of your components. At the moment I use option 1 until I figure out a more elegant solution.
Hope this helps

@Brocco I would agree with some of the folks regarding this issue and consider reopening it. Providing a way to utilize the mixins would be great.

Any updates about this?

If anything we need a global mixin file we can put variables and mixins that will be available to all components but not need reimporting over and over.

@zurie re: "we need a global mixin file":  It might be nice, but is it a need?  You can get that with minimal repetition by having a _common.scss that imports the variable and mixin partials you want, then import that where you need it.  So its just one line, and you opt-in.
I'm still puzzling through all of this, but it seems to me there's no problem (other than build time) if we want to import a partial that has no rules, just mixins, variables, and functions.  They don't generate any css (unless used).  The problem is, we want to use something for which bootstrap hasn't provided a mixin, (and/or its in a file that also contains rules).  What we want (I think) is an import-for-extend-only.  This would be a "safe" import, that would only contribute mixins, functions, and variables.  In this context, @extend would not add to rules, but would need to write new ones, based on the imported scss.
However, this would be a sass feature, not angular-cli.  Barring something like that getting added (or maybe it exists and I just don't know about it?), I guess we need to use some variation of that suggested by @jessepinuelas. Either that, or look at what bootstrap does, and write your own mixins, as needed.

@mconner I think you're misunderstanding the problem; the issue isn't that these mixins/whatever need to be explicitly imported to be used. The problem is that importing them into your component styles puts a copy of the imported stuff in your component style, leading to pointlessly bloated style bundles.
Bootstrap styles are global, but there's no way to include them in an encapsulated component without encapsulating those imports as well. @jessepinuelas's solution might work, but it bypasses the encapsulation.

@spongessuck There are two issues here, the simpler of these was the explicit imports, to which @zurie was referring :

If anything we need a global mixin file we can put variables and mixins that will be available to all components but not need reimporting over and over.

But yes, the more complicated is the issue of bloat.  However, as I said initially, mixins do not contribute anything to css. And Bootstrap separates the mixins from rules. So, if you import:
@import '~bootstrap/scss/functions';
@import '~bootstrap/scss/variables';
@import '~bootstrap/scss/mixins';

instead of:
@import '~bootstrap/scss/bootstrap';

you will get no css generated (unless you actually use a mixin, and then you get what you used).  I just tried it to confirm.
On the other hand, if you want to extend a rule, you have to import the file containing the rule, and that will, of course, pull in that rule (which we want) and any others (which we don't want).  So, if you want encapsulation without bloat, prefer mixins. And maybe write your own if the the vendor doesn't provide one.  I think.  As I said, I'm still puzzling through this.

Are there any more developments, or ideas, on how to import the most minimal amount to use a mixin within a component's .sass or .scss file?

@mconner's solution is the best I've come across after much searching. I went further and created my own _mixins.scss file where I put the imports, and then in my components I just do:
@import "mixins";
whenever I need any of them.

@jkossis @luisaceituno @bentolor
We have faced similar issue recently. Using vuejs it is super convenient to use bootstrap mixins and class definitions for our own classes. The problem we have faced is that the resulting CSS was including bootstrap code for every component (only on dev)
Basically solution for us was to artificially create "import once" functionality during the SASS processing. We did the following:

Split our global scss into two parts: First, that includes the bootstrap and some global styles and second that includes the first one and have only one "split" class that we name uniquely
Add intermediate webpack loader after the SCSS task to remove all css before the "split" class
Use the second sass file in all components and use the first only once in the Main module.

Hope that helps.


Update index.md

Added h2 to h6 headings to the code (lines 28 to 32) and added a description about headings (line 49).

[x] I have read freeCodeCamp's contribution guidelines.
[x] My pull request has a descriptive title (not a vague title like Update index.md)
[x] My pull request targets the master branch of freeCodeCamp.
[x] None of my changes are plagiarized from another source without proper attribution.
[x] My article does not contain shortened URLs or affiliate links.

If your pull request closes a GitHub issue, replace the XXXXX below with the issue number.
Closes #XXXXX

You checked a box indicating "My pull request has a descriptive title (not a vague title like Update index.md)", but you have given it the exact name "Update index.md". Descriptive titles and good explanations of changes are necessary for a well maintained codebase. Free Code Camp is choosing to relax some of its requirements to help people reach their Hacktoberfest goals, but this PR would normally be rejected. In the future, please help repository maintainers by providing useful titles and do not check off requirements without reading and following them.


Refactor/warn case sensitive modules plugin

What kind of change does this PR introduce?
ES5 => ES6 refactor
Did you add tests for your changes?
refactor, all existing tests pass
If relevant, link to documentation update:
N/A
Summary
Helping in part of the ES6 refactor
Does this PR introduce a breaking change?
Nope
restored from #3722

Reviewed: Unittest need to be fixed.


Update LingView so prepare-upgrade can be run manually

By commenting, I expect prepare-upgrade will be triggered for now?

Done a while ago, possibly in commit 9153f0b


Medical Refactor

The mythical medical refactor. Heard whispered in hushed tones in the corners of the contributor channels, it is indeed REAL! If you listen very carefully you might be able to hear it's cry.... "Just a week away"
About the PR

This is a complete rewrite of the wounds branch, focusing on making it easier to integrate in forks while being more extensible. I also simplified the logic a bit and added some helper functions for body system.
Progress checklist

[X] Implement placeholder bodypart targeting. (Randomly select a bodypart)
[X] Wounds can be created on body parts from specified damage types
[X] Wounds have severity that scales effects
[X] Wounds are selected based on the % of maximum part health in damage
[X] Wounds can be added or removed
[X] Wound system utilizes the current damage system and damage types to create wounds
[X] Implement wound hp/integrity systems. (HP heals, integrity does not)
[ ] Wounds properly support attaching/detaching/destroying bodyparts
[X] Wound creation can be canceled by blocking an event
[X] Convert humanoid crit and death states to use consciousness to trigger instead of direct damage
[x] Implement pain, acts as a softcap on the number of wounds. If pain is above certain thresholds the entity enters crit or dies. Most wounds cause pain. Pain can be modified to increase or decrease it's affect.
[x] Wounds heal over time and healing rate can be adjusted
[x] When a wound is healed it may change to another wound (a scar)
[x] Damage from a wound can overflow onto surrounding parts if it is destroyed
[X] Rewrite bleeding system to be per-wound based instead of just a hp buffer
[X] Pain, and wound components can be modified by treatments
[X] Implement consciousness and consciousness modifier system. With thresholds for crit and death.
[X] Convert crit and death states to use consciousness to trigger instead of direct damage
[ ] Update defibrillator to provide a temporary consciousness buff instead of healing damage
[ ] Update medical scanner to list wounds and general health
[ ] Create easy to use API/Events for all wound-med features
[ ] Implementing blocking flag for consciousness if a player has been dead long enough to prevent consciousness changes/reviving if the player has been dead long enough
[x] Implement critical parts for consciousness.
[ ] Update projectiles to have special wounding logic (create penetration wounds + embedded projectiles)
[ ] Hook up treatments to usable actions/doafters on items
[ ] Implement Icons/Screen effects for Pain
[ ] Implement Icons/Screen effects for Bloodloss
[ ]  Implement Icons/Screen effects for Consciousness
[ ] When a part is destroyed, it amputates any connected parts and applies an amputation wound to the parent part.
[ ] Setup all medical items to use the new treatment system
[ ] Pain and wound components can be modified by reagents
[x] Map all damage types to traumas
[ ] Balance medical items
[ ] Balance wounds and damage types
[ ] Rough rebalance of all weapons/attacks
[ ] Implement explosion damage logic
[ ] Implement wound cauterization (Heat damage has a chance to stop bleeding on parts)
[ ] Implement wound stacking/reopening
[ ] Implement medical inspection text logic
[ ] Implement medical inspection prompt (A right click option to get a medical description of an entity)
[ ] Add hotkey for medical inspect

Why / Balance

Technical details

Media

[ ] I have added screenshots/videos to this PR showcasing its changes ingame, or this PR does not require an ingame showcase

Breaking changes

This is going to cause issues with custom damage types and weapon damage or humanoid armor will need to be re-balanced to work properly with wounds. Additionally, any new races will need to be setup with either the common humanoid traumas or require custom traumas to be created. If woundables are not present on a humanoid it will never enter crit or death
Changelog

@deltanedas This is the updated medical branch, I'm still working on porting the stuff over from the other branch but it's quite a bit simpler and easier to use. Sorry for leaving the old branch sitting for so long, I needed a break from coding for a while since work was getting hectic. It's calmed down alot now so I'm able to work on this more :)
I'll try to keep you up to date in the discussion as I add/rewrite functionality

Pog, someone's doing surgery again

just a week away

Just a week away...

Just a week away...

Just a week away…

Has it realy been a week...

Guys, it will be people.  The person may have personal problems.   I'm going to remove the submodule update so that the tests don't break

Just a week away…

I work full-time as a AAA game developer, I work on SS14 in my spare time (Which has been a bit limited as of late). "Just a week away" is a meme, not a promise and it will get done when it gets done. Medical gameplay is complex and touches many systems, which means it takes time to code (especially if you want to do it properly).

requires #20202

Github you're tripping. Don't worry I'm still working on this 😅 Not sure why it got closed

looks like this is still being worked on, which is nice, but are there any status updates? not to pressure you though

Currently working through getting core features done, I just finish healing and pain and now I'm going to be working on some of the UI/UX, basically creating a barebones medical menu and some moodlets/icons to give the player critical information about pain/consciousness and general damage.

After that I'm probably going to be refactoring bleeding and asphyxiation because they are both spaghetti and rely on the old damage system.

what does the medical UI look like? an MS paint drawing will do

what does the medical UI look like? an MS paint drawing will do

Sorry I didn't get a notification for this 😓.
I don't have a UI layout yet for the medical UI, my best description would be very similar to how Rimworld's medical UI works: https://www.reddit.com/media?url=https%3A%2F%2Fpreview.redd.it%2F7r7g9h16ptd71.png%3Fauto%3Dwebp%26s%3D0e1200ad7ff9665ec44a380068c233091bb1cfd6

what does the medical UI look like? an MS paint drawing will do

Sorry I didn't get a notification for this 😓. I don't have a UI layout yet for the medical UI, my best description would be very similar to how Rimworld's medical UI works: https://www.reddit.com/media?url=https%3A%2F%2Fpreview.redd.it%2F7r7g9h16ptd71.png%3Fauto%3Dwebp%26s%3D0e1200ad7ff9665ec44a380068c233091bb1cfd6

this is RimHUD

I don't have a UI layout yet for the medical UI, my best description would be very similar to how Rimworld's medical UI works

take your time (i didnt get a notification because i was banned :trollface:)

if you have played baystation 12, is this similar to that?

is this out of scope?

The idea is similar to baymed, but it's different. The closest system would probably be rimworlds.
And yeah, rollerbeds/medical equiptment/interactions is generally out of scope for this PR, it's big enough as is 😓

theres always another PR

oooh different blood types? sweet! will there be something like nanoblood from baystation that can be given to all races and blood types? and how do you choose or know what your blood type is? putting it in character notes (C menu) would be cool because that isn't used for anything other than antagonist objectives

if this is what i think it is, you could just combine the current medical inspect with the current regular inspect, so you dont have to click that heart icon

are hands and arms separate like paradise/baystation or will they be linked together like TG?

are hands and arms separate like paradise/baystation or will they be linked together like TG?

I hope it's separate, it'd be weird taking both arm/hand damage for certain things like touching electrified doors.

id prefer them connected, its weird to separate it since i doubt there'll be any point in needing to work on BOTH

the one downside to this is that i cant troll chemists by asking them for tramadol (but peridaxon will still not exist)

id prefer them connected, its weird to separate it since i doubt there'll be any point in needing to work on BOTH

separate is better for RP servers, combined is better for LRP. but I prefer separate

Hand and arms as separate bodyparts, but you will (most likely) be able to remove an arm with hands attached. Blood types will likely be stored on your PDA/med scanner/medDB

will cyborg limbs act as prosthetics?

You should try to make it so that most tools and some materials can act as prosthetics, at least for lrp. It could allow cool stuff like a Jaws of life arms, or would let me give a clown 4 peg legs and arms after they slip me one too many times lmao.

item arms would be so epical

Waiting on #21916 to get merged before I can continue working on bloodstream/body chemistry stuff.

just another week

just another week

Let
The man
COOK

just another week

Let
The man
COOK

as an open note to everyone, stop leaving pointless comments on the PR. this is the development github, not your shitpost board.

Currently working on refactoring metabolism reactions and bloodstream in another branch. I'm going to merge that first since this PR is big enough already. Just because you don't see progress on the main PR doesn't mean it isn't happening (also I have a day job >.>). And like Emo said, this is for development discussion not shitposting, I don't want to have to make the discussion contrib only. Thank you!

Hi there. Is there anything we can do to help on this matter?

Hi there. Is there anything we can do to help on this matter?

Hey, sorry for the late reply! I'm still working on core functionality/design at the moment so additional hands won't be much help at the moment. But as soon as the core stuff is finished I'll make a post letting people know how they can help :) Thank you for your interest though, I really appreciate the offers of support from people it's just that right now having more people working on it would just make things more complicated and take longer.
For anyone wondering about the lack of updates to this pr, Don't worry! I'm still working on this but the changes are on another branch: Jezithyr:MedicalRefactorV2 specifically. Once I've reached feature parity with the code on this branch I'm going to force push over it with the cleaned-up code.

There we go, so this branch should now be up to date with the changes I have been making. If you're working on something that you are worried might conflict in medical, the code in this PR will now be up to date so you can take a look!

Most important thing, that we need. I wish you success.

Just in case anyone is wondering: No, NewMed is not being affected by the feature freeze. It should be ready before the freeze starts and if not work will still continue on it during the freeze.

i got a bit of objection with renaming the chemist role because they already DO all that stuff you say that they will do, because that's their job already. it would just be a name change for the sake of changing the name. please, tell me if im wrong here.

i got a bit of objection with renaming the chemist role because they already DO all that stuff you say that they will do, because that's their job already. it would just be a name change for the sake of changing the name. please, tell me if im wrong here.

I concur, "chem" just sounds better than "pharm", and honestly do we truly expect people to spell pharmacist correctly without autocorrect :trollface:

i got a bit of objection with renaming the chemist role because they already DO all that stuff you say that they will do, because that's their job already. it would just be a name change for the sake of changing the name. please, tell me if im wrong here.
It's not just a change for the sake of changing the name, the role of the chemist is going to be changing to be more involved in treatment, especially during surgery. Pharmacist is more in-line with what the role would actually be doing which is creating/mixing drugs, overseeing how they get used, and managing drug mixtures during surgery.

It's a departure from what traditional SS13/14 chemistry is where you just make the pills, hand them to the doctor and anything after that isn't your problem. Pharmacists will be actively involved in treating patents while also managing the stockpiles of drugs. Tbh some of the larger stations might still end up having a chemist role to handle up-keeping the drug supply while the pharmacists can handle the application part.
The main reason for the rename is that the gameplay around drugs will be changing considerably and as such chemists can no longer just sit in the back of med making drugs all day (Unless there are other pharmacists to handle the medicating part)

Honestly I like the name pharmacist better and yeah it kinda fits better. The idea of freeing chemists from purely being stuck in chemistry and taking a more active role in treatment by actively assisting in using the drugs they procure, as well I can foresee them being surgical assistants with keeping the patient sedated and well medicated.
I will miss calling the chemists "chemmy" though

It also indicates that you shouldn't be making space drugs all shift and instead should be making healing chemicals 😆

I don't like the pharmacist title, chemists will still be chemists and make the drugs. The whole giving drugs part can be done over the counter like how it is already, so I don't think game play changes drastically from that distributing part of being a chemist. This does feel like changing

i got a bit of objection with renaming the chemist role because they already DO all that stuff you say that they will do, because that's their job already. it would just be a name change for the sake of changing the name. please, tell me if im wrong here.

It's not just a change for the sake of changing the name, the role of the chemist is going to be changing to be more involved in treatment, especially during surgery. Pharmacist is more in-line with what the role would actually be doing which is creating/mixing drugs, overseeing how they get used, and managing drug mixtures during surgery. Although some of the larger stations might still end up having a chemist role to handle up-keeping the drug supply while the pharmacists are busy dealing with treating patients.
It's a very large departure from what traditional SS13/14 chemistry is where you just make the pills, hand them to the doctor and anything after that isn't your problem. Pharmacists will be actively involved in treating patents while also managing the stockpiles of drugs. (Effectively: Doctor -> treats injuries and performs surgery. Pharmacist -> gives people drugs to manage recovery/pain relief/sedation.)
The main reason for the rename is that the gameplay around drugs will be changing considerably and as such chemists can no longer just sit in the back of med making drugs all day (Unless there are other pharmacists to handle the medicating part)

The way you describe it makes it sound like those are just optional RP aspects of being a chemist but not a required part of the job. You could have a medical assistant do the exact same thing what you're describing the chemist can do with overseeing drug distribution to manage recovery. Makes it sound like a rename for rename's sake but maybe the gameplay will further show the difference.

I don't like the pharmacist title, chemists will still be chemists and make the drugs. The whole giving drugs part can be done over the counter like how it is already, so I don't think game play changes drastically from that distributing part of being a chemist. This does feel like changing

Think of it more as a more descriptive job title. A pharmacist is making the medical prescriptions that people need while a chemist is experimenting with the chemicals to find new drugs. In ss13 goon it was like this, 2 jobs with chems where medical had the name of pharmacist while sci had chemist. It makes the name of the job closer to how they will function in the refactor too.

The way you describe it makes it sound like those are just optional RP aspects of being a chemist but not a required part of the job. You could have a medical assistant do the exact same thing what you're describing the chemist can do with overseeing drug distribution to manage recovery. Makes it sound like a rename for rename's sake but maybe the gameplay will further show the difference.

It's a rename to make it clearer to players (especially new ones) what the intended job of that role is. Pharmacist makes it more clear that it's specifically related to medical and not just a role about locking yourself in the back room and making meth all shift.
Another reason for the renaming of the role is that eventually, I'd like to implement chemistry-based research for science and "Chemist" would suit that role better since it's focused on producing/researching general purpose chemicals that aren't related to medical.

Seems like the week away is getting closer

I mostly had tought about first implementing limb mechanics and then implementing the entire New sistem, still nice

could not build to try it out yesterday, but the pr promisse is great! kudos

good.

I think the reason they were using animal organs was so they could heal from eating raw meat and blood without dying

Lizards intentionally have animal organs

organ transplant soon

This is hype inducing

I think the reason they were using animal organs was so they could heal from eating raw meat and blood without dying since they convert into uncooked animal proteins

I explained why this isn't a good reason to inherit form animal Organs instead of human/humanoid Organs in discord. But it mainly boils down to the fact that a lizard Organ has more in common conceptually with a human Organ than a corgi/hamster Organ.

heal from eating raw meat and blood without dying since they convert into uncooked animal proteins

This functionality comes from a single component that already exists on humanOrgans. Instead of copying animal organs (and thus creating a case where Lizards don't use the same BaseOrgans as every other humanoid), we should instead inherit from humanOrgans and override the values to allow lizards to eat blood/raw meat.
Don't worry, I'm not changing how lizard digestion works. I'm just fixing this really dumb inheritance structure so that it actually makes sense. (Most AnimalOrgans won't have the same level of simulation as full humanoids for performance reasons)

Soon, ill get arrested by turning someone into a papa johns

Would be cool to have meds that have to be sprayed into the wounds.
Like spraying space cleaner on burns for inmideatly increase the pain

Slowly seeing how all squares get filled :D

Add estrogen and testosterone

surgery. just a week away

please god, provide me peak…

Over a month of no commits. It's joever

Newmed isn't dead, I just need to refactor parts of chemistry before I can continue. Starting with: #30254 .

Part 1 of the chem refactor is converting Reagents/Reactions to Entities and adding support for creating/removing/modifying reagents/reactions during gameplay. This is something that will be shared/co-developed with RMC-14 (they need the functionality for creating randomized chems).
Part 2 of the chem refactor is updating the solutions API to be less complicated/convoluted to use, moving solutions to use fixedpoint4 for more precision, and implementing rate limiting for reactions. Once part 2 is done, work can continue on NewMed since it will be no longer held back by missing chem functionality.

So people may be worrying that newmed is dead after the discord message I posted about it being paused. I'm here to tell you that is no longer the case! The issues that I mentioned have been resolved and I'm back to working on both the chem-refactor and newmed!

also @godtierprogrammer thank you for the reviews I really appreciate someone putting in the effort to read through my frankly kludged together code :P . At this point the newmed branch is mainly prototyping and not really something I'd consider ready for review, there is a whole bunch of half-finished/slapped together stuff that is there to prototype out the functionality rather than serve as the actual implementation.
Part of the reason why I started working on refactoring chemistry is because I realized while prototyping newmed that there was no way to get the functionality I needed from chem/reactions without creating some truly awful hackery. Not to mention that any time I ended up interacting with reagent/solution code I wanted to lay down and cry 😭. What I'm trying to say is that chances are large parts of NewMed are going to get rewritten pre-merge anyways so there isn't really much point to doing reviews at this stage unless you really want to (I won't stop you! Just know that I might end up rewriting that code anyways 😅).

You may be worrying that newmed is dead after the discord message I posted about it being paused. I'm here to tell you that is no longer the case! The issues that I mentioned have been resolved and I'm back to working on both the chem-refactor and newmed!

Nice to see you okay again

the name of the PR sounds like a business
NewMed, Implementing Medical Gameplay into SS14 since 2023. New product in under a week!

When 😭

Due to various reasons I have decided to no longer directly be involved
with the development of wizden, and as such I will be closing this PR.
Please do not reopen this PR as I will be continuing work on this PR elsewhere.
However I give permission for anyone to upstream these changes when they
are completed. I'd like be clear that this is just to avoid dealing with merge conflicts coming from
two different versions being developed at the same time, and not a decision made of spite.
A more in depth explanation can be found here: https://discord.com/channels/310555209753690112/770682801607278632/1321967056446554174

Hope for serious development from wizden has faded....

Hope for serious development from wizden has faded....

Not if you listen to what she said.  shes still working on newmed, just on a different fork, someone just needs to port it over to wizden when she's done.


Files with zero mtime are treated as nonexistent

On a CentOS 7 system:
$ tar xf ome-cmake-superbuild-0.1.0.tar.xz
tar: ome-cmake-superbuild-0.1.0/cmake/GitVersion.cmake: implausibly old time stamp 1970-01-01 01:00:00
$ ls -l ome-cmake-superbuild-0.1.0/cmake/GitVersion.cmake
-rw-r--r-- 1 rleigh lsd 236 Jan  1  1970 ome-cmake-superbuild-0.1.0/cmake/GitVersion.cmake

This source archive was packed incorrectly, and a single file has an mtime of zero (i.e. start of Unix epoch).
$ mkdir build
$ cd build
$ cmake -G Ninja ../ome-cmake-superbuild-0.1.0
$ ninja
[1/1] Re-running CMake...
[cmake re-run repeated 100 times]
ninja: error: manifest 'build.ninja' still dirty after 100 tries

The cause:
$ ninja -d explain
ninja explain: output /tmp/ome-cmake-superbuild-0.1.0/cmake/GitVersion.cmake of phony edge with no inputs doesn't exist

Clearly, the file does exist.  If I touch /tmp/ome-cmake-superbuild-0.1.0/cmake/GitVersion.cmake to update the timestamp to the current time, then ninja runs perfectly.  It appears to be the case that Ninja is treating files with a zero mtime as being nonexistent, which seems incorrect.  Please could you consider changing that behaviour?  While this is an unusual corner case--files shouldn't have a timestamp this old--it can and does happen for various reasons, and it would be nice if Ninja could work correctly under these circumstances.
Kind regards,
Roger

bool Cleaner::FileExists(const string& path) {
  string err;
  TimeStamp mtime = disk_interface_->Stat(path, &err);
  if (mtime == -1)
    Error("%s", err.c_str());
  return mtime > 0;  // Treat Stat() errors as "file does not exist".
        ^^^^^^^^^
}

Looks like a likely candidate.  And
  void MarkMissing() {
    mtime_ = 0;
  }

Also RealDiskInterface::Stat and DiskInterface::MakeDirs.  Fundamentally, you're using mtime as both a timestamp and an error code, and that's simply not possible given that all valid mtime values are also valid timestamps (even negative ones).  This assumption is even stated in src/disk_interface.h:
  /// stat() a file, returning the mtime, or 0 if missing and -1 on
  /// other errors.

It's assumed in a few other places as well.  I don't think your current strategy is safe, particularly when zero is often the default when unspecified in e.g. archive formats.  Consider that the underlying stat(2) syscall returns ENOENT when a directory entry is not existent, independently of st_mtime.  Maybe you could use a small Stat structure containing a valid flag or enum (-1 and 0 cases) plus the mtime.  It could provide operator< for mtime comparison and operator(bool) or operator! for validity checking and comparison, which would provide both features unambiguously and remove the problematic behaviour observed here.

This is a dupe of #795. Someone suggested to let an mtime of 1 instead of 0 mean "file does not exist" :-)
On #795, the latest status is that it's now easy to print a message like "ninja can't handle files with mtime of 0" instead of confusingly and silently marking them as dirty. Would that be good enough for you?
Why is the file's mtime 0 on your system?

mtime of 1 is a clever hack!  Could also just add one to all mtimes, to
make 0 available.  (We don't care about the specific value of the mtimes,
they're just a magic cookie that changes when the file changes.  On Windows
they're some other random value, not a epoch-seconds-counter.)
On Tue, Apr 5, 2016 at 6:36 PM, Nico Weber<EMAIL_ADDRESS>wrote:

This is a dupe of #795 https://github.com/ninja-build/ninja/issues/795.
Someone suggested to let an mtime of 1 instead of 0 mean "file does not
exist" :-)
On #795 https://github.com/ninja-build/ninja/issues/795, the latest
status is that it's now easy to print a message like "ninja can't handle
files with mtime of 0" instead of confusingly and silently marking them as
dirty. Would that be good enough for you?
Why is the file's mtime 0 on your system?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/ninja-build/ninja/issues/1120#issuecomment-206067067

The reason for the mtime being zero in this specific instance is by using the Python tarfile module without explicitly setting the mtime on the TarInfo objects being saved into the tar file--it defaults to zero unless set by hand.  This is different to the behaviour of the analogous interface in the zipfile module, which defaults to the current system time (as might be expected when creating a new file).  But I've seen other tools also do similar things.
Using an mtime of 1 would indeed be a "clever hack" and would certainly work around the issue here.  On the other hand, a fix such as I suggested would be not much more effort and would remove the need for any special values at all, making it robust under all circumstances.  Either way would be fine with me.
Thanks for following this up,
Roger

But the 0 mtime is never intentional, right? It seems like erroring out might be the best way forward here.

It's not intentional, no.  But it is something which occurs relatively commonly by accident.  I don't think erroring out would be appropriate; if anyone ever makes a source release with an unset timestamp, that's de facto unbuildable with ninja.  Setting it to 1 would be a much nicer strategy.

The Windows code to convert the filesystem-representation timestamp into a
Ninja TimeStamp type is here:
https://github.com/ninja-build/ninja/blob/master/src/disk_interface.cc#L60
I'm proposing changing this code:
https://github.com/ninja-build/ninja/blob/master/src/disk_interface.cc#L193
to return st.st_mtime+1;
I believe it will solve everything at the cost of us not properly handling
files with a date of the maximal epoch (but we'll have all sorts of y2k38
problems in that area once we're anywhere near that).
On Fri, Apr 8, 2016 at 7:56 AM, Roger Leigh<EMAIL_ADDRESS>wrote:

It's not intentional, no. But it is something which occurs relatively
commonly by accident. I don't think erroring out would be appropriate; if
anyone ever makes a source release with an unset timestamp, that's de facto
unbuildable with ninja. Setting it to 1 would be a much nicer strategy.
—
You are receiving this because you commented.
Reply to this email directly or view it on GitHub
https://github.com/ninja-build/ninja/issues/1120#issuecomment-207466847

I am seeing something similar here when building inside a flatpak sandbox, basically all the files handled by (ostree)[https://ostree.readthedocs.io/en/latest/], meaning the "system" files have a mtime = 0 (not sure why they do that, but I think it is related to the file de-duplication sytstem). Those system files are not considered nonexistent but as "dirty", and full rebuild is triggered each time I am running ninja in that env.
When rebuilding with ninja -d explain it says that the files are dirty for example:
ninja explain: /usr/include/glib-2.0/gio/gtcpwrapperconnection.h is dirty

For flatpak I'm going to add the patch in https://github.com/flatpak/flatpak/issues/607#issuecomment-287952697 to our ninja build until this is figured out.

I think I have a more thorough fix in #1293

Just noting also, I did also file Patrick Griffis' patch used in Flatpak as pull request #1292, which is a good workaround for the problem, which we know is well tested (builds regularly pass with that applied at least).
But I did put some effort to do it thoroughly in #1293 so that the semantic is changed and any mtime is a valid one, including 0 and negative mtimes which should also be valid (not sure how the codebase stands up to negative mtimes, though). While I think #1293 is the correct fix, it is of course a bit more of an invasive change and would be good to review that (I did not end up modifying deps_log.[cc,h], I'm not sure if a change is needed there to distinguish between file existence and mtimes). I have however built a lot of GNOME modules using the #1293 branch without issue.
Whichever route is taken, it would be nice to have some fix in upstream ninja for this.

Thanks, I merged your #1292. As said elsewhere, this looks like the superior fix to me, so I'll ignore #1293 for now if that's ok with you.

And thanks much for the fix!

Thanks, I merged your #1292. As said elsewhere, this looks like the superior fix to me, so I'll ignore #1293 for now if that's ok with you.

No worries, I'm just happy that I don't need a downstream patch :)
Maybe you can also close #795


Adds module for getting  the longitude at nearest midnight.

Adds module for getting nearest the longitude at nearest midnight.
Need to specify the observation date and the latitude.
I haven't added tests for the Sunspotter class because I am planning on making a separate Sunspotter Object on a different PR and would need to refactor code here.

Hello @Raahul-Singh! Thanks for opening this PR.

In the file pythia/cleaning/midnight_rotation.py:

Line 8:1: E302 expected 2 blank lines, found 1
Line 77:101: E501 line too long (107 > 100 characters)
Line 82:101: E501 line too long (101 > 100 characters)
Line 154:101: E501 line too long (113 > 100 characters)
Line 179:101: E501 line too long (113 > 100 characters)
Line 206:101: E501 line too long (113 > 100 characters)
Line 236:101: E501 line too long (113 > 100 characters)
Line 249:48: W291 trailing whitespace
Line 267:101: E501 line too long (113 > 100 characters)
Line 269:101: E501 line too long (137 > 100 characters)

In the file pythia/cleaning/tests/test_midnight_rotation.py:

Line 17:52: E226 missing whitespace around arithmetic operator
Line 17:92: E226 missing whitespace around arithmetic operator
Line 17:101: E501 line too long (116 > 100 characters)
Line 25:52: E231 missing whitespace after ','
Line 25:101: E501 line too long (114 > 100 characters)
Line 26:101: E501 line too long (114 > 100 characters)
Line 27:101: E501 line too long (115 > 100 characters)
Line 28:101: E501 line too long (115 > 100 characters)
Line 29:101: E501 line too long (114 > 100 characters)
Line 30:101: E501 line too long (117 > 100 characters)
Line 35:63: E231 missing whitespace after ','

Do see the Hitchhiker's guide to code style

Thanks @dpshelio !


Fixing foxy CI

Codecov Report

:exclamation: No coverage uploaded for pull request base (foxy@4c89b1b). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             foxy    #157   +/-   ##
======================================
  Coverage        ?   3.59%
======================================
  Files           ?      13
  Lines           ?     947
  Branches        ?     152
======================================
  Hits            ?      34
  Misses          ?     843
  Partials        ?      70

Flag
Coverage Δ

unittests
3.59% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c89b1b...0be6707. Read the comment docs.


Format in validation length will cause error

In primitives lines 116 - 119
if obj.minItems and len(self) < obj.minItems:
            raise errs.ValidationError('Array should be more than {0}, not {1}'.format(o.minItems, len(self)))
if obj.maxItems and len(self) > obj.maxItems:
            raise errs.ValidationError('Array should be less than {0}, not {1}'.format(o.maxItems, len(self)))

o is not defined and should be replaced to obj

thanks for pointing out, there must be some test case missed.

thx for reporting issues, this issue is fixed in v0.8.9.


tailwindcss-intellisense throws cssConflict for irrelevant classNames

Firstly thank you for this module, it's quite needed for a decent size codebase with TailwindCss
As shown in the screenshot example, tailwindcss-intellisense warns me about a cssConflict while the offending element className has no relation with the one used with twMerge.

Hey @ThaJay! 👋
I think this is an issue with the Tailwind CSS Intellisense VS Code extension. tailwind-merge can't highlight anything in VS Code since it doesn't have a VS Code extension.
It's probably better if you open an issue in the repository of the VS Code extension: https://github.com/tailwindlabs/tailwindcss-intellisense

No worries. 😊


PostgreSQL support

Is it possible to save the data into PostgreSQL instead, so that tag embedding can be done and compared between Danbooru and Derpibooru?
Cross-referencing: https://github.com/fire-eggs/Danbooru2019/tree/master/database and https://derpibooru.org/pages/data_dumps
Goal: https://www.aclweb.org/anthology/L18-1156.pdf

Certainly! makedb.py creates the database with basic SQL commands. That program can be modified  using a Postgres package (e.g. psycopg2) and the appropriate Postgres-specific SQL commands.
Looking at the schema from Derpibooru, it looks like it would be fairly straightforward to use it:

Derp's "public.images" looks like a close match to makedb's "images" table.
Derp's "public.tags" corresponds to makedb's "tags" table. You'd have to assign some appropriate value for the 'slug' and 'category' columns, based on Derp's data dump.
Derp's "public.tag_changes" appears to be the correspondent for makedb's "imageTags" table. In Gwern's metadata, only the image_id and tag_id columns are available; you'd need to assign appropriate values for the other columns.

Gwern's metadata doesn't readily permit any of the other tables in the Derpibooru schema to be populated. I'm unsure if you need any of those tables for your goals.

Also I have discovered something:

https://danbooru.donmai.us/wiki_pages/help:tag_implications (for reference usage)
https://danbooru.donmai.us/posts?tags=help%3Atag_aliases (might be needed for data cleaning)
these two are not in the DB schema atm.

History suggests the Danbooru server itself was using PostgreSQL. If that is still the case, asking the Danbooru maintainers for a copy of the database would be the most expeditious method for PostgreSQL support.
I don't have PostgreSQL and don't have the background to support it.

A possible solution: https://github.com/bitdotioinc/pgsqlite


defaultzonepath parameter defined, but not used in template name.conf.erb

dns::defaultzonepath is defined as a parameter, however setting this has no effect to named.conf.

I'm not entirely sure that's correct. It is used here:
https://github.com/theforeman/puppet-dns/blob/47fb22b2812302167b0003bc1479346ffa3b199c/templates/named.view_header.erb#L50-L54

Yes true, it is used as part of views configuration. But it should also be used as a global setting under named.conf.


Best practices for working with multiple emitters?

Sorry to be such a noob at this, but as I've never worked with particles before this is all new to me! I've got 3 or 4 cool effects I'm using - 3 are burst type things that only run once when something happens, and 1 is an ongoing bubble generator that runs all of the time.
Do I need to do something to clean up the "run once" emitters? Is it ok to just keep creating them over and over again, or should I create them all in the beginning, and then start and stop them?
thanks in advance for any advice!

Because the emitters reuse their particles, it is better to keep the "run once" emitters for the next time you need them. When you no longer need them, you should call their destroy() method to make sure that they and all their particles get garbage collected.

Thank you, that is really helpful.


[FEATURE] use JDBC driver meta data to determine object case

Is your feature request related to a problem? Please describe.
From #216 - using Java/JDBC DatabaseMetaData api Tableau could determine the case scheme for a backend. E.g.:

https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#storesLowerCaseIdentifiers()
https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#storesMixedCaseIdentifiers()
https://docs.oracle.com/javase/7/docs/api/java/sql/DatabaseMetaData.html#storesMixedCaseQuotedIdentifiers()
etc.

This could be used to avoid the need to manually specify CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING.
This may be more of a Desktop enhancement than SDK, opening here for comment/feedback
Describe the solution you'd like
Use JDBC driver reported information to avoid need for CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING
Describe alternatives you've considered
Other option would be to use the config .ini file and use the LogicalQueryFormat setting
About You
Name: Chris Clark
Company: Actian Corp

tfs 1197844

Closing this enhancement request after internal backlog priority review of JDBC related work.
The current public documentation of CAP_JDBC_BIND_DETECT_ALIAS_CASE_FOLDING is all that is planned at this time.


Add OpenAI provider

This PR adds support for OpenAI.
Not sure if you accept 3rd party providers. If not, this PR can be used as a reference for people to integrate OpenAI.
Pre-launch Checklist

[x] I read the Contributor Guide and followed the process outlined there for submitting PRs.
[x] I read the Tree Hygiene wiki page, which explains my responsibilities.
[x] I read the Flutter Style Guide recently, and have followed its advice.
[x] I signed the CLA.
[ ] I listed at least one issue that this PR fixes in the description above.
[x] I updated/added relevant documentation (doc comments with ///).
[ ] I added new tests to check the change I am making, or this PR is test-exempt.
[x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Please run dart format

fixed

Hey @davidmigloz, sorry to do this to you. Could you please build a single merged version of these three PRs? We want to see that all three can be successfully merged together into main. Thanks!
/cc @mit-mit @csells

Done! Closing this one in favor of https://github.com/flutter/ai/pull/30


BUG: IDE warns about import that can't be resolved

While implementing #3, I was left with a displeasing solution in the way I test:
from fastapi.testclient import TestClient
from main import app # <- here

def test_boilerplate():
    with TestClient(app) as client:
        ...

main.py is technically not accessible, so this causes two issues:

It's unclear what we're importing
It confuses the IDE

It is not a bug per se, as pytest.ini in the root directory specifies where the files to test lies and it provides the necessary context at this point for the Python code to be valid.
I will still be looking into a fix/change for this as I dislike the pattern

Decided to change the structure of the project, and went for:
snippy
├── src
│   ├── source.py
│   └── tests
│      └── tests.py
├── .gitignore
├── LICENSE
├── README.md
├── ...

which removes the IDE warning, gives the import sense, and makes me sleep better at night


Original pre-0.7 compilation

What is your question?
Hi, I was wondering if there would be any point in the future where you could possibly add back or just make the original source code pre-0.7 optional? I think some of the changes are cool, but the removal of stuff like getProperty and setProperty just feel foolish to do. Issues like these with the current compilation are the reason why I only use 0.6.3. Like, MY mod can't work because of removals like this?! I put work into this! (I'm not mad, I'm just asking a question: will it come back at any point in time? its a yes or no question)

getProperty and setProperty still exist???
also you can use this

ok yea I can use that but I use a lot of normal get/setProperty instead of get/setPropertyFromClass, is there any way to deal with that? sry if it's obvious bc I'm not on my computer rn

No.
Who would ever want to go to the hell that is pre-0.7 source.

setProperty and getProperty did NOT get removed, and I can't make the source "optional", that's not how coding works.


Can null references be replaced with empty lists/strings and still not be written out?

This would be ideal as PhenoPacket will return a lot of nulls in most cases - check comments in PhenoPacket.java

cc @kshefchek - want consistent behavior with the python

I found a solution for empty lists (see #49). Instead of using empty strings, what about using java.util.Optional for those fields that are allowed to be empty? Nice to be able to make this explicit. I tried making a field into an Optional<String> and the serialization "just worked" as desired.

@kshefchek any word on this for the Python version?

@balhoff How will using Optional help with deserialising and using the phenopacket after creation? Won't there be Optional everywhere and a heap of Optional.isPresent() guards? It's still a bit of an alien thing to me so please point out the benefits!

@julesjacobsen I think it is only useful if you can reliably NOT bother checking for null for return types that are not wrapped in Optional. Wrapping in Optional just forces the null check for the cases when it's expected that a value might not be there. But I haven't used it much in Java. In Scala, Option is super common, but there is a culture of disallowing use of null. Since Option is a monad in Scala you get really some special syntax for composing multiple Options and using them in pattern matching. So maybe without the extra language support they're not that big of a win in Java.

OK, so just returning an empty value for cases which could be null as in empty Collection or String should be OK from the API. These won't be written out in the serialised data and the builders will ensure they are never null in the case of the Entity or Condition, however I'll need to look at how this will work in the case of variables internal to Entity and Condition, especially ClassInstance.


[LLC] Implement LLC generator support for ETag and MatchConditions in LLC APIs

The ETag type can be found in two places in our APIs:
As the value of a request header in a conditional request (RFC7232) => these translate to method input parameters in LLC
If an operation takes:

All of If-Match, If-None-Match, If-Modified-Since, and If-Unmodified-Since (this is unusual in MS REST APIs)

The method generated for the operation will take a single RequestConditions type as parameter, allowing each of these values to be set as the appropriate type

Both of If-Match and If-None-Match

The method generated for the operation will take a single MatchConditions type as a parameter, allowing each value to be set as an ETag

One of either If-Match or If-None-Match

The corresponding parameter type for the header will be ETag
We have general guidelines stating that we use friendly boolean values with convenient names indicating the semantics of the operation in our APIs.  These will be added as part of the grow-up story, but not in generated code.
In a model body
We don’t generate models for LLC, however, when we generate models for the grow-up story, any field in the model with the name “ETag” (case-insensitive as this varies across REST APIs) will be generated as a property of type ETag, and is the responsibility of the service to validate.
This was designed in this issue: https://github.com/Azure/azure-sdk-for-net/issues/24853

Hi @annelo-msft
Current if in the swagger defined If-Match, If-None-Match, If-Modified-Since, and If-Unmodified-Since, LLC codegen will generate method with them as parameter. e.g in AppendBlobClients
internal HttpMessage CreateCreateRequest(string containerName, string blob, long contentLength, int? timeout, string blobContentType, string blobContentEncoding, string blobContentLanguage, byte[] blobContentMD5, string blobCacheControl, IDictionary<string, string> metadata, string leaseId, string blobContentDisposition, string encryptionKey, string encryptionKeySha256, string encryptionScope, DateTimeOffset? ifModifiedSince, DateTimeOffset? ifUnmodifiedSince, string ifMatch, string ifNoneMatch, string ifTags, string blobTagsString, DateTimeOffset? immutabilityPolicyExpiry, string immutabilityPolicyMode, bool? legalHold)
So this issue want to combine all those 'If-Match, If-None-Match, If-Modified-Since, and If-Unmodified-Since,' header parameters into one model 'RequestConditions', and pass this model as one parameter of the method instead.

PR: https://github.com/Azure/autorest.csharp/pull/1669


Navigate through history

When clicking on a link or jumping to a search result, it would be nice if you could go back to where you were before following the link, by pressing some shortcut or clicking a button in the toolbar.
History is implemented in pdfjs (https://github.com/mozilla/pdf.js/pull/2921) but I haven't checked if it is accessible in ng2-pdfjs-viewer.

I am currently working on rewriting the browser code in Kotlin and migrating to a custom build of PDF.js, so this will be implemented when the new version will be ready.

I am currently working on rewriting the browser code in Kotlin and migrating to a custom build of PDF.js

What's the status on this? Is there anything I can do to help? I was thinking about working on the PDF viewer (resolving issues, adding minor things that I came across while using it in combination with TeXiFy) but I feel like things will be a lot easier once this refactor is done. Both in terms of maintenance and resolving merge conflicts/porting changes to Kotlin later.
So if there is anything I can do, please let me know and I will be more than happy to help :)

@slideclimb I am sorry for such a long silence from me - I had a lot of stuff to do. So, the main work on the refactor is done - most of the features are working now. I also added an implementation of structure view, as many people have asked me for it.
Currently, the broken features are:

Presentation mode (it is completely broken as it requires a lot of work on the IDE side to support it)
Document info
Error reporting from browser

I'm glad that you want to help - it means a lot to me :) If you want to do something, you can take a look at the kotlinjs-migrate branch - I think there will be no major changes to the current code, so you can safely use it. There will be only minor changes to the master branch migration branch is not merged.
Even with some of the useful stuff broken, I am thinking of releasing an alpha version of the plugin in its current state, just for testing purposes.

@FirstTimeInForever No worries! That sounds pretty good already. I will definitely have a look at that branch, and I'll actually use it to see if anything else is broken. Releasing an alpha is a good idea, I'm pretty sure @PHPirates would use that as well.
You'll hear from me if I think I can be useful to fix any of the broken things, or if I decide to work on anything else. Anyways, I'm so excited that you're migrating this to Kotlin JS, so thank you for going through the hassle that comes with such a big refactor :)

@FirstTimeInForever If I runIde on 4b972f4 (on Windows) then I get java.io.FileNotFoundException: Could not load resource file: \\web-view\index.html\, is that work in progress or am I missing something?
java.io.FileNotFoundException: Could not load resource file: \\web-view\index.html\
	at com.firsttimeinforever.intellij.pdf.viewer.utility.PdfResourceLoader.loadFromRoot(PdfResourceLoader.kt:37)
	at com.firsttimeinforever.intellij.pdf.viewer.jcef.PdfStaticServer.sendInternalFile(PdfStaticServer.kt:79)
	at com.firsttimeinforever.intellij.pdf.viewer.jcef.PdfStaticServer.process(PdfStaticServer.kt:46)
	at org.jetbrains.io.DelegatingHttpRequestHandler$process$1.invoke(DelegatingHttpRequestHandler.kt:21)
	at org.jetbrains.io.DelegatingHttpRequestHandler$process$2.test(DelegatingHttpRequestHandler.kt:35)
	at org.jetbrains.io.DelegatingHttpRequestHandler$process$2.test(DelegatingHttpRequestHandler.kt:16)
	at com.intellij.openapi.extensions.impl.ExtensionProcessingHelper.lambda$findFirstSafe$0(ExtensionProcessingHelper.java:46)
	at com.intellij.openapi.extensions.impl.ExtensionProcessingHelper.computeSafeIfAny(ExtensionProcessingHelper.java:56)
	at com.intellij.openapi.extensions.impl.ExtensionProcessingHelper.findFirstSafe(ExtensionProcessingHelper.java:46)
	at com.intellij.openapi.extensions.ExtensionPointName.findFirstSafe(ExtensionPointName.java:50)
	at org.jetbrains.io.DelegatingHttpRequestHandler.process(DelegatingHttpRequestHandler.kt:34)
	at org.jetbrains.io.DelegatingHttpRequestHandlerBase.messageReceived(DelegatingHttpRequestHandlerBase.kt:15)
	at org.jetb

This is definitely a bug. I'll try to fix it as soon as possible.

@PHPirates pathing issue should be fixed by f92f341d1c612f8736b9e7ff610993ea302c4c3e.

I can confirm it fixed, thanks!

Is this history navigation supposed to work when clicking a link which goes to a page on the internet? The Go Back button isn't doing anything for me. (0.12.0-alpha.4) It works when staying within the pdf for browsing between pages, but the problem is that when I click on an Internet link the only way to go back is to close the pdf and open it again.

Fixed in 0.12


Add SemaDB VST wrapper

Description: Adding vectorstore wrapper for SemaDB.
Issue: None
Dependencies: None
Twitter handle: semafind

Checks performed:

[x] make format
[x] make lint
[x] make test
[x] make spell_check
[x] make docs_build

Documentation added:

SemaDB vectorstore wrapper tutorial

@baskaryan That's all updated with checks running locally.

looks great, thanks @nuric!


.env variables not working when putenv() is disabled

Laravel Version: 6.3.0
PHP Version: 7.3.10
Database Driver & Version: not applicable

Description:
When putenv() function is disabled (some hosting providers do this because of security reasons) the env() funcion doesn't detect the variable value. This is because dotenv can't put the environment variable using putenv() and env() relies on getenv()...
Must Laravel throw an exception here? If Laravel relies on putenv()/getenv() and is not avaliable the developer must know that if .env exists and is not empty the variables will not be loaded.
Steps To Reproduce:

Add putenv to disabled_functions in php.ini and restart webserver
Check any value on .env with env('VARIABLE_NAME');

Suggested solution
Then, the fix can be added to the env() function with a fallback to check $_ENV global variable instead of returning the default value:

function env($key, $default = null)
    {
        $value = getenv($key);

        if ($value === false && array_has($_ENV, $key) {
            $value = array_get($_ENV, $key);
        } elseif ($value === false) {
            return value($default);
        }

        // ...

        return $value;
    }

I can make a pull request if you think this solution is acceptable.

When I add \Illuminate\Support\Env::disablePutenv(); to the AppServiceProvider I can still use the env helper properly so it's not required.

Thanks @driesvints, didn't know this method because is not documented.
Anyway, I think this must be detected and disabled automatically if putenv() is not available. Is just a simple check with function_exists(). Why use an adapter that will fail?

@driesvints, Is this issue definitely closed? We should not discuss whether it is necessary for Laravel to automatically call \Illuminate\Support\Env::disablePutenv(); if putenv() is not available.


Hint color is almost unreadable

Hello! I find it very hard to read the hint color:

Is it possible to change it to a lighter color/more visible value? Can it be configured?
Love your library! I've been using it to write https://deno.land/x/deno_changesets

Ah, duplicate of https://github.com/c4spar/deno-cliffy/issues/237
I'll close this one.

Not realy a duplicate since #237 is related to the command module.
I will reopen it and try a different color for the hint.
I also thought about adding a theme option to make it posible to customize all colors like this:
const theme = {
  hint: colors.brightBlue.italic,
  // ...
};

await Input.promt({
  message: "Select a module",
  theme,
});

The theme can be then re-used for all prompts.


refactor(dataplanes): move getStatusAndReason into data transformation file

Follow up to https://github.com/kumahq/kuma-gui/pull/1800. Follow up PRs to this one: https://github.com/kumahq/kuma-gui/pull/1834 and https://github.com/kumahq/kuma-gui/pull/1835.
Moves the getStatusAndReason dataplane utility into its data transformation file.
Signed-off-by: Philipp Rudloff<EMAIL_ADDRESS>

I also expanded the test cases for the status utility a bit more to take into account the difference between standard proxies and gateways. For gateways, we shouldn’t check the inbound field. Specifically the inbounds.length === 0 check resulting in a gateway being 'online' is incorrect.


Slow packing times for GPTQ

System Info
optimum=1.20.0, python=3.11.9, torch=2.3.1+cu121, system=ubuntu 20.04

Who can help?
No response
Information

[ ] The official example scripts
[X] My own modified scripts

Tasks

[X] An officially supported task in the examples folder (such as GLUE/SQuAD, ...)
[ ] My own task or dataset (give details below)

Reproduction (minimal, reproducible, runnable)
I'm quantizing opt-350m using gptq. The actual quantization is fast but then packing layers is slow. The code to quantize the models is as follows:
quant_config = GPTQConfig(
    bits=args.bits,
    group_size=128,
    tokenizer=tokenizer,
    dataset=["c4-new"],
)

quantized_model = AutoModelForCausalLM.from_pretrained(
    "facebook/opt-125m",
    quantization_config=quant_config,
    torch_dtype=torch.bfloat16,
    trust_remote_code=True, # Remove later
    device_map="cuda:0"
)

I added some timings to GPTQ packing (https://github.com/huggingface/optimum/blob/5c803db8cef21b22d0bdbf8a69653b74656e193e/optimum/gptq/quantizer.py#L614):
import time
for name in qlayers:
    logger.info(name)
    start = time.time()
    quantizers[name], scale, zero, g_idx = quantizers[name]
    # so far can only pack layer on CPU
    layer_device = qlayers[name].device
    qlayers[name].to("cpu")
    layers[name], scale, zero, g_idx = layers[name].to("cpu"), scale.to("cpu"), zero.to("cpu"), g_idx.to("cpu")
    qlayers[name].pack(layers[name], scale, zero, g_idx)
    qlayers[name].to(layer_device)
    print(f"Time to pack {name}: {time.time() - start}")

This has timings:
Time for transformer.blocks.0.attn.Wqkv: 0.24
Time for transformer.blocks.0.attn.out_proj: 0.08
Time for transformer.blocks.0.ffn.down_proj: 0.25
Time for transformer.blocks.0.ffn.up_proj: 91.95
However, if I run it in parallel (which I think preserves everything) as:
from concurrent.futures import ThreadPoolExecutor
import time

def pack_layer(name):
    logger.info(name)
    start = time.time()
    quantizers[name], scale, zero, g_idx = quantizers[name]
    layer_device = qlayers[name].device
    qlayers[name].to("cpu")
    layers[name], scale, zero, g_idx = layers[name].to("cpu"), scale.to("cpu"), zero.to("cpu"), g_idx.to("cpu")
    qlayers[name].pack(layers[name], scale, zero, g_idx)
    qlayers[name].to(layer_device)
    print(f"Time for {name}: {time.time() - start}")

with ThreadPoolExecutor() as executor:
    executor.map(pack_layer, qlayers.keys())

The timings are
Time for transformer.blocks.0.attn.Wqkv: 0.21
Time for transformer.blocks.0.attn.out_proj: 0.10
Time for transformer.blocks.0.ffn.down_proj: 0.22
Time for transformer.blocks.0.ffn.up_proj: 3.64
Expected behavior
Whats strange is that the packing time is so long, taking ~ 90 seconds to pack the up projection. Whats even stranger is that the individual packing times per layer are lower when run in parallel (not just the overall time). Ie when run sequentially the time to pack the up proj is 90 seconds but this goes down to 3 seconds when running packing each layer in parallel.

@zankner I have fixed this in both GPTQModel and back-ported fix to main of AutoGPTQ.

Awesome thanks!

@Qubitium sorry to re-open but still needs to be ported into optimum.


for models from a registry other than HF, we need to follow symlinks.

Checklist:

[ ] Commit Message Formatting: Commit titles and messages follow guidelines in the
conventional commits.
[ ] Changelog updated with breaking and/or notable changes for the next minor release.
[ ] Documentation has been updated, if necessary.
[ ] Unit tests have been added, if necessary.
[ ] Functional tests have been added, if necessary.
[ ] E2E Workflow tests have been added, if necessary.

Opened up an issue for testing here: https://github.com/instructlab/instructlab/issues/2405 Feel free to add anything to the issue or reassign it accordingly.

@Mergifyio backport release-v0.19


docs: add AWS S3 instructions to SPA fallback

Added example for AWS S3 via CloudFront

Thanks, but I'm going to refrain from adding any more examples on how to handle the SPA fallback on different servers because this got out of control in the previous docs.

That's a pitty, because a static website hosted on S3 is pretty common usecase for a SPA imho


[Hermanos de Sangre] Doesn't appear the sheet

Hello,
I've uploaded the sheet from "Hermanos de Sangre" 24 days ago and it isnt available in create new game. Could you tell me about what happens?
Thanks!

I have the same problem to another sheet. Good luck, friend!


FutureWarning:  Method .ptp is deprecated and will be removed in a future version. Use numpy.ptp instead.

Can you provide any context on when this warning appears?

iris = px.data.iris()
import plotly_express as px
tips = px.data.tips()
gapminder = px.data.gapminder()
election = px.data.election()
wind = px.data.wind()
carshare = px.data.carshare()
px.scatter(tips, x="total_bill", y="tip", facet_row="time", facet_col="day", color="smoker", trendline="ols",
category_orders={"day": ["Thur", "Fri", "Sat", "Sun"], "time": ["Lunch", "Dinner"]})

Hmm ok. can you tell me which versions of Python, pandas and numpy you're using?

I'm getting the error using version 1.16.4 of numpy and 3.7.3 of python
In [78]: sys.version
Out[78]: '3.7.3 | packaged by conda-forge | (default, Jul  1 2019, 22:01:29) [MSC v.1900 64 bit (AMD64)]'

Any help?

@emmanuelle can you take a look at this one please? We see this warning sometimes depending on the version of statsmodels and numpy... it would be good to just figure out which recent versions of these dependencies avoid these issues...

FYI https://github.com/statsmodels/statsmodels/commit/18dcceefbfed61b0d11ae5884a9f4cea82c2edb5

pandas (>= 0.24.0) raises this warning.
https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.ptp.html

I think using pandas (>= 0.24.0) and statsmodels (<= 0.9.0) causes this warning.

We can reproduce the warning like this:
import numpy as np
import pandas as pd

s = pd.Series([0])
np.ptp(s)

This is because np.ptp(s) calls s.ptp.
https://github.com/numpy/numpy/blob/v1.17.0/numpy/core/fromnumeric.py#L2430-L2496

It seems this line causes the warning.
https://github.com/statsmodels/statsmodels/blob/e2d768ae8da848dc46f63fae3059c5c4139007e0/statsmodels/tsa/tsatools.py#L103

One way to avoid the warning is convert a pandas series to a numpy array like below:
- fit_results = sm.OLS(y, sm.add_constant(x)).fit()
+ fit_results = sm.OLS(y, sm.add_constant(x.values)).fit()

https://github.com/plotly/plotly.py/blob/4a30dc66230269aad008650544dd1045a40b1896/packages/python/plotly/plotly/express/_core.py#L236

Ah, thanks for all the detective work @harupy ! Indeed just adding .values there makes the warning go away :) I'll patch it tonight.

@nicolaskruchten
Can I create a PR for this?

@nicolaskruchten You're welcome!

thanks so much @harupy yes this works much appreciated. thank you @nicolaskruchten for asking this question

This was bothering me to no end on a variable assignment. As someone new to Python, it was quite discouraging as I though I was doing something wrong. @harupy , thanks for your help - you just saved me from despair! hontou ni arigatou gozaimashita!


Support for minikube or docker daemon alternative?

Apologies if this is not the correct place to ask this question.
With many teams moving away from docker entirely due to the recent changes to docker desktop - will spring-native support a container runtime other than docker? If not - are there any recommended ways of working around this to use a non-docker container runtime?
Thanks

Spring Native uses the Spring Boot build plugins for building Docker images. Spring Boot supports using minikube with the Maven or Gradle plugin. Other Docker-compatible daemons might work in the same way.

Thanks for your feedback @scottfrederick.


[Feature] Build on save

Add support for 'Build on save'.

After we have check on save and keyboard shortcut for build I think this is obsolete.

@KalitaAlexey unrelated, but previously you had a patch where it would automatically build examples if the open file was an example file, that made the Build Example commands obsolte, do you have any plans to submit that patch again?

@Draivin No, because it is not good to build on save. Because it can be very slow. When you want to build or run you can press shortcut.


Only pull languages from Transifex that we're interested in.

Also adds a new command, pull_all(), that pulls all languages, regardless of "reviewed" status, down from Transifex.

Coverage remained the same at 41.28% when pulling 680a009b2f96dfbb3cee0025a9b10439a4941230 on sarina/limit-languages into 0f6abacfb96a7a8884b3e0bf8e7489c7d4c6a6c5 on master.


Universal GitHub Enterprise Server support?

Hi! I saw the notes/slides from your talk that got posted to the Git mailing list. If I understand them correctly, you're trying to get GitLab (and have successfully gotten Gitea) to recognize your client ID out of the box so that authentication to any GitLab server, not just gitlab.com, works.
I think the same issue applies to GitHub, in that there's a self-hosted GitHub Enterprise Server product. For example, the gh command supports talking to an enterprise server, not just github.com.
Have you chatted with GitHub about doing the same thing? I think it'd be useful - my employer ended up writing something that's basically like your tool for our Enterprise Server instance. (Though at this point we've customized it significantly and we now support transparent single sign-on without even bringing up a browser, which is its own much more complicated story.... but I think it'd be useful for other people! :) )

Hi. I haven't yet, but I'd like to. I don't have access to any GitHub Enterprise Server for testing.
Some questions about GitHub Enterprise Server:

Does Git Credential Manager OAuth work without setup? It looks like GCM uses universal OAuth client credentials for all instances.
How does GitHub Enterprise Server respond to unauthenticated requests? Is the www-authenticate: Basic realm="GitHub" header the same as GitHub?

Does Git Credential Manager OAuth work without setup? It looks like GCM uses universal OAuth client credentials for all instances.

GCM uses different redirect URIs for GitHub Enterprise Server and GitHub  so the client registrations must be independent in some way

How does GitHub Enterprise Server respond to unauthenticated requests? Is the www-authenticate: Basic realm="GitHub" header the same as GitHub?

The same according to https://github.com/git-ecosystem/git-credential-manager/issues/1246

As a workaround, you can try impersonating Git Credential Manager:
# impersonate Git Credential Manager
git config --global credential.https://github.example.com.oauthClientId 0120e057bd645470c1ed
git config --global credential.https://github.example.com.oauthClientSecret 18867509d956965542b521a529a79bb883344c90
git config --global credential.https://github.example.com.oauthRedirectURL http://localhost/

This requires git-credential-oauth installed from HEAD and Git 2.41 or later.

I just tested the workaround and can confirm it works.

GitHub Enterprise Server 3.8.3
git version 2.42.0
git-credential-oauth current main ea99a5e


Add explicit whitelist of RPC commands for luigid

Limit calls on Scheduler from API to only those required for functionality.
Basic functionality testing was conducted, but not extensive testing.

nice!

Nice! Thanks :)


Charging Station - Provide human being name

Provide an editable name for a charging station ID and:

Display it in all lists (meaning lookup with charging station collections in all the DB storages)
Use it in filtering and search
Add it in export
Ensure performance is kept acceptable
Use it in SAP Analytics? Check in all the app...

If that means that we can have two identifiers for humans and the same object, to avoid confusion for operators that might use the charging station admin interface, the two ids if different should be displayed at least in the charging station IHM details.
And maybe also provide a way to sync the two ids when possible.

No we have only one ID and a name which can have dups.

If that means that we can have two identifiers for humans and the same object, to avoid confusion for operators that might use the charging station admin interface, the two ids if different should be displayed at least in the charging station IHM details.
And maybe also provide a way to sync the two ids when possible.

No we have only one ID and a name which can have dups.

That's what I mean by identifier for human: users can have one name on the dashboard and an other on the charging station. In the case they are different, both should be displayed on the details, so operators can check if they are logged on the right charging station web interface to troubleshoot.


Consider higher-level abstraction over the OpenAPI info in Kptfile

The OpenAPI is extremely verbose, and not human readable and writable, and the user has to rely on CLI porcelain for the most part (CLI becomes the defacto API)

We no longer need OpenAPI definitions for setters in kpt v1.


[$500] Expense rule - Tags can not be selected when Employee create Expense rule

If you haven’t already, check out our contributing guidelines for onboarding and email<EMAIL_ADDRESS>to request to join our Slack channel!

Version Number: 1.4.6-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:
Precondition:

On Old dot Admin should create a tag called "Test Tag"
Steps:
On OldDot - Login as the employee
Navigate to Account Settings > Expense Rules
Try to select "Test Tag"
admin<EMAIL_ADDRESS>employee<EMAIL_ADDRESS>
Expected Result:
Tag dropdown should be selectable
Actual Result:
When click on Tag field - empty dropdown list opens. User can not see "Test Tag"
Workaround:
Unknown
Platforms:

Which of our officially supported platforms is this issue occurring on?

[ ] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[ ] iOS: mWeb Safari
[x] Windows: Chrome
[ ] MacOS: Desktop

Screenshots/Videos
Add any screenshot/video evidence

View all open jobs on GitHub
Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01ddde5a6eeb482175
Upwork Job ID:<PHONE_NUMBER>626617344
Last Price Increase: 2023-12-01

Can external contributors access the Expensify Stackoverflow? I have not seen anything in the contributing guidelines about how to gain access.

Contributor details
Your Expensify account email<EMAIL_ADDRESS>Upwork Profile Link: https://www.upwork.com/freelancers/~0143cbafce457c37fd

@lanitochka17 I am not able to recreate this -

Logging into<EMAIL_ADDRESS>there isn't a tag called test at this time.