Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Changed Target Architecture to x64 #2254

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Alexander-Sol
Copy link
Contributor

@Alexander-Sol Alexander-Sol commented Feb 20, 2023

mzLib only targets x64 processor architecture. Targeting "Any CPU" introduced warning messages when building.

This PR switches MetaMorpheus to only target x64 architecture. Warnings are no longer generated on build.

This may fix issue #2253, but I wouldn't hold my breath.

@Alexander-Sol Alexander-Sol changed the title Wix test2 Changed Target Architecture to x64 Feb 24, 2023
@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #2254 (1f0815a) into master (a21ba5d) will decrease coverage by 0.10%.
The diff coverage is 30.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2254      +/-   ##
==========================================
- Coverage   91.14%   91.05%   -0.10%     
==========================================
  Files         133      133              
  Lines       20064    20094      +30     
  Branches     2797     2801       +4     
==========================================
+ Hits        18288    18297       +9     
- Misses       1280     1298      +18     
- Partials      496      499       +3     
Impacted Files Coverage Δ
TaskLayer/MbrAnalysis/MbrAnalysisResults.cs 82.96% <30.00%> (-15.21%) ⬇️
EngineLayer/MetaMorpheusEngine.cs 87.64% <0.00%> (+0.28%) ⬆️

Copy link
Contributor

@acesnik acesnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I think keeping it to x64 is a more consistent with the build, as you mentioned.

I think there are maybe some unintentional changes to look into before merging.

<RemoveFolder Id="MetaMorpheusProgramMenuFolder" On="both" />
</Component>
</Fragment>
<!--Shorthand way of getting the install directory-->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes intentional?

@@ -15,12 +15,13 @@ public class MbrAnalysisResults
public readonly ConcurrentDictionary<ChromatographicPeak, MbrSpectralMatch> BestMbrMatches;
public readonly FlashLfqResults FlashLfqResults;
private Dictionary<string, List<string>> PeptideScoreDict;
public bool MaxQuantAnalysis { get; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes intentional in this file, too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants