-
Notifications
You must be signed in to change notification settings - Fork 1.8k
KBABANKñETWORK Online YOUÜ*83 84\Ñetwotrk AR3RT. #3138
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
base: dependabot/npm_and_yarn/ejs-3.1.7
Are you sure you want to change the base?
KBABANKñETWORK Online YOUÜ*83 84\Ñetwotrk AR3RT. #3138
Conversation
The script type property is no longer needed and generates a warning when the HTML is validated using browser plugins.
|
Do you want me to review this PR? Please comment |
|
** Mayor. Appreciate and Approval.&(ฯ220 84**DexterWern8.θθ01*<^.YOUTUB3.COM |
DexterWernJr
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HelloMONET. 1
|
/review |
|
PR Summary: This PR updates CI/CD workflows and release scripts while fixing typos and naming inconsistencies. Key changes include: • Adjusted release workflow defaults and commands (e.g. npm vs npx versus yarn, switching branch names from main to master). |
| run: | | ||
| git log -1 --stat | ||
| git push origin main --tags | ||
| git push origin master --tags |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL_BUG] Changed branch name in 'git push' (line 126) from 'main' to 'master'. Verify that this matches the repository's default branch to avoid deployment issues.
| push: | ||
| branches: | ||
| - main | ||
| - master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[CRITICAL_BUG] Branch name updated from 'main' to 'master' on line 6. Ensure consistency across all workflows and repository settings.
| public static get getMultiRootWorkspaceName(): string { | ||
| return Config.getSettings<string>('multiRootWorkspaceName'); | ||
| } | ||
|
|
||
| public static setMutiRootWorkspaceName(val: string) { | ||
| public static setMultiRootWorkspaceName(val: string) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NITPICK] Renamed getters and setters from 'MutiRoot' to 'MultiRoot' (lines 120 and 124). This fix improves naming clarity; ensure all references are updated.
| import { Config } from './Config'; | ||
|
|
||
| export const SUPPRORTED_EXT: string[] = [ | ||
| export const SUPPORTED_EXT: string[] = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NITPICK] Corrected the misspelled variable name from 'SUPPRORTED_EXT' to 'SUPPORTED_EXT'. This change improves readability and consistency.
| let ServerInstance = require('live-server').start(params); | ||
| setTimeout(() => { | ||
|
|
||
| if (!ServerInstance._connectionKey) { | ||
| return callback({}); | ||
| } | ||
|
|
||
| httpShutdown(ServerInstance); | ||
| require('http-shutdown')(ServerInstance); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[PERFORMANCE_OPTIMIZATION] The usage of dynamic require calls (e.g. for 'live-server' and 'http-shutdown') inside functions (lines 8 and 15) may lead to repeated module resolution. Consider moving these require statements to the module scope to benefit from caching.
const liveServer = require('live-server');
const httpShutdown = require('http-shutdown');
export class LiveServerHelper {
static StartServer(params, callback) {
setTimeout(() => {
try {
let ServerInstance = liveServer.start(params);
setTimeout(() => {
if (!ServerInstance._connectionKey) {
return callback({});
}
httpShutdown(ServerInstance);
return callback(ServerInstance);
}, 1000);
} catch (err) {
console.error(err);
callback({
errorMsg: err
});
}
}, 0);
}
static StopServer(LiveServerInstance, callback) {
LiveServerInstance.shutdown(() => {
// callback(); /*only Working first time, Unknown Bug*/
});
LiveServerInstance.close();
liveServer.shutdown();
setTimeout(() => { callback(); }, 1000);
}
}| const announcement = packageJSON.announcement; | ||
|
|
||
| if (!announment && Object.keys(announment).length === 0) return; | ||
| if (!announcement && Object.keys(announcement).length === 0) return; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[NITPICK] Corrected variable name from 'announment' to 'announcement' and updated its usage (lines 8-10). Confirm that the 'opn' package is still supported or consider using its modern alternative if available.
|
Reviewed up to commit:428e01caf02bfa7ee75741df0f02fc9d2b5b0999 Additional SuggestionOthers- The 'liveServer.settings.useBrowserPreview' setting has been removed. Confirm that its removal is intentional and update any documentation or related logic accordingly. |
PR Type NSA**
Jkearneymck69@Gmail.com ŒNEÑET
What kind of change does this PR introduce?
[ ] Bugfix [ ] Feature [ ] Refactoring (no functional changes, no api changes) [ ] Documentation content changes [ ] Other: <!-- Please describe: -->What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information